Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate semconv codegen to weaver #5793

Draft
wants to merge 8 commits into
base: main
Choose a base branch
from

Conversation

jsuereth
Copy link

@jsuereth jsuereth commented Sep 6, 2024

This is a prototype for discussion and demonstration in next week's Go SiG meeting.

This migrates semconv codegeneration off the (deprecated, soon to be abandoned) semconvgen to weaver.

A few things in this PR:

  • We're using Weaver's automatic git resolution of tags for consuming semconv.
  • We're using Weaver's acronym/synonym features instead of opentelemetry-go-build-tools
  • We're directly forking docker within your makefile, including no longer running as root.

I couldn't find (easily) the documentation you had previously for generating semconv (which I did find in the collect). This should make it as easy as:

make semconv-generate TAG=v1.27.0

which is how I generated the v1.27.0 code here.

Regarding JINJA formatting, I didn't have a chance to go through whitespace refactoring/fun. For other codegen we run formatting tools as part of the makefile. I've manually run go fmt on the generated code, but we would likely want to have that a part of the Makefile to automatically correct Jinja's wierd whitespacing.

Copy link

codecov bot commented Sep 6, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 84.6%. Comparing base (29c0c28) to head (6b6f9e1).
Report is 5 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #5793     +/-   ##
=======================================
- Coverage   84.6%   84.6%   -0.1%     
=======================================
  Files        272     272             
  Lines      22759   22778     +19     
=======================================
+ Hits       19255   19271     +16     
- Misses      3164    3166      +2     
- Partials     340     341      +1     

see 1 file with indirect coverage changes

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

1 participant