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

[SemanticConvention] Migrate generation to Weaver and bump version to 1.26 #2040

Merged

Conversation

joaopgrassi
Copy link
Member

@joaopgrassi joaopgrassi commented Sep 5, 2024

Fixes #1944

Changes

  • Move code generation to Weaver instead of build-tools
  • Bump semconv version to v.1.26.0 as that's required for the Weaver move

My goal with this initial PR is to do the minimum changes to get Weaver to work. To bring this package to stabilization, we will need more PRs to separate stable to unstable attributes.

Merge requirement checklist

  • CONTRIBUTING guidelines followed (license requirements, nullable enabled, static analysis, etc.)
  • Unit tests added/updated
  • Appropriate CHANGELOG.md files updated for non-trivial changes
  • Changes in public API reviewed (if applicable)

@joaopgrassi joaopgrassi requested a review from a team September 5, 2024 13:19
@joaopgrassi
Copy link
Member Author

To maintainers:

Can we not run the Run python3 ./build/scripts/sanitycheck.py for the Semconv package? For ex, now it is complaining with trailing space which is not so easy to fix within code gen. Also, given this is auto gen, seems a bit superfluous.
image.

Also the non-ascii check is problematic. Will try to remove that from the semconv itself, but I still believe we can just skip this check for this package.

@joaopgrassi
Copy link
Member Author

The error failures are because if missing xml docs for some old, deprecated otel attributes. I already fixed it in semconv.

Missing XML comment for publicly visible type or member 'OtelAttributes.AttributeOtelLibraryName'

For now, I will remove them from generation and can be added later once a new semconv version is released with my fix

They don't have a brief which causes warnings with public API.
Will re-add once a new semconv release is out.
@joaopgrassi joaopgrassi changed the title Migrate semantic conventions generation to Weaver Migrate semantic conventions generation to Weaver and bump version to 1.26 Sep 6, 2024
@cijothomas
Copy link
Member

To maintainers:

Can we not run the Run python3 ./build/scripts/sanitycheck.py for the Semconv package? For ex, now it is complaining with trailing space which is not so easy to fix within code gen. Also, given this is auto gen, seems a bit superfluous. image.

Also the non-ascii check is problematic. Will try to remove that from the semconv itself, but I still believe we can just skip this check for this package.

I think it is reasonable to modify the sanitycheck script to have a "ignore" list of directories to ignore and ask it to ignore sem conventions. (This can be done as a separate PR and merge before this PR to keep the overall scope focused)

@joaopgrassi
Copy link
Member Author

joaopgrassi commented Sep 10, 2024

I think it is reasonable to modify the sanitycheck script to have a "ignore" list of directories to ignore and ask it to ignore sem conventions. (This can be done as a separate PR and merge before this PR to keep the overall scope focused)

Alright, thanks @cijothomas! I will look into it and send a separated PR.

Edit: PR is there, let me know if that's something you had in mind
#2060

@github-actions github-actions bot added the infra Infra work - CI/CD, code coverage, linters label Sep 10, 2024
Copy link

codecov bot commented Sep 10, 2024

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 72.54%. Comparing base (71655ce) to head (b9c8e3f).
Report is 427 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #2040      +/-   ##
==========================================
- Coverage   73.91%   72.54%   -1.38%     
==========================================
  Files         267      314      +47     
  Lines        9615    11829    +2214     
==========================================
+ Hits         7107     8581    +1474     
- Misses       2508     3248     +740     
Flag Coverage Δ
unittests-Exporter.Geneva 53.26% <ø> (?)
unittests-Exporter.InfluxDB 94.65% <ø> (?)
unittests-Exporter.Instana 71.24% <ø> (?)
unittests-Exporter.OneCollector 94.32% <ø> (?)
unittests-Exporter.Stackdriver 75.73% <ø> (?)
unittests-Extensions 88.57% <ø> (?)
unittests-Extensions.AWS 83.41% <ø> (?)
unittests-Extensions.Enrichment 100.00% <ø> (?)
unittests-Instrumentation.AWS 84.78% <ø> (?)
unittests-Instrumentation.AWSLambda 88.92% <ø> (?)
unittests-Instrumentation.AspNet 77.00% <ø> (?)
unittests-Instrumentation.AspNetCore 85.27% <ø> (?)
unittests-Instrumentation.ElasticsearchClient 79.87% <ø> (?)
unittests-Instrumentation.EntityFrameworkCore 55.49% <ø> (?)
unittests-Instrumentation.EventCounters 76.36% <ø> (?)
unittests-Instrumentation.GrpcNetClient 79.61% <ø> (?)
unittests-Instrumentation.Hangfire 93.58% <ø> (?)
unittests-Instrumentation.Http 82.05% <ø> (?)
unittests-Instrumentation.Owin 85.79% <ø> (?)
unittests-Instrumentation.Process 100.00% <ø> (?)
unittests-Instrumentation.Quartz 78.94% <ø> (?)
unittests-Instrumentation.Runtime 100.00% <ø> (?)
unittests-Instrumentation.SqlClient 90.90% <ø> (?)
unittests-Instrumentation.StackExchangeRedis 69.92% <ø> (?)
unittests-Instrumentation.Wcf 48.91% <ø> (?)
unittests-PersistentStorage 65.78% <ø> (?)
unittests-Resources.AWS 77.93% <ø> (?)
unittests-Resources.Azure 82.35% <ø> (?)
unittests-Resources.Container 72.41% <ø> (?)
unittests-Resources.Gcp 72.54% <ø> (?)
unittests-Resources.Host 73.94% <ø> (?)
unittests-Resources.OperatingSystem 77.20% <ø> (?)
unittests-Resources.Process 100.00% <ø> (?)
unittests-Resources.ProcessRuntime 77.08% <ø> (?)
unittests-Sampler.AWS 87.61% <ø> (?)

Flags with carried forward coverage won't be shown. Click here to find out more.

see 342 files with indirect coverage changes

@Kielek Kielek changed the title Migrate semantic conventions generation to Weaver and bump version to 1.26 [SemanticConvention] Migrate generation to Weaver and bump version to 1.26 Sep 13, 2024
Will be done in a follow up
open-telemetry#2068
@Kielek Kielek merged commit 2f76c24 into open-telemetry:main Sep 13, 2024
212 of 213 checks passed
@joaopgrassi joaopgrassi deleted the feat/semconv-migrate-to-weaver branch September 13, 2024 08:42
@Kielek Kielek mentioned this pull request Sep 13, 2024
2 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
infra Infra work - CI/CD, code coverage, linters
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Migrate semantic conventions generation to Weaver
5 participants