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

Hangfire: Downgrade OpenTelemetry.Api.ProviderBuilderExtensions to 1.6.0 to avoid MS.Extensions.* v8 dependencies #1502

Merged
merged 5 commits into from
Dec 19, 2023

Conversation

gao-artur
Copy link
Contributor

@gao-artur gao-artur commented Dec 19, 2023

Fixes #1500.

Changes

Temporarily downgrading the OTel dependency. The version will be restored after the instrumentation release.

For significant contributions please make sure you have completed the following items:

  • Appropriate CHANGELOG.md updated for non-trivial changes
  • Design discussion issue #
  • Changes in public API reviewed

Copy link

codecov bot commented Dec 19, 2023

Codecov Report

Attention: 3 lines in your changes are missing coverage. Please review.

Comparison is base (71655ce) 73.91% compared to head (6fa6cd9) 80.55%.
Report is 101 commits behind head on main.

Additional details and impacted files

Impacted file tree graph

@@            Coverage Diff             @@
##             main    #1502      +/-   ##
==========================================
+ Coverage   73.91%   80.55%   +6.64%     
==========================================
  Files         267      113     -154     
  Lines        9615     3055    -6560     
==========================================
- Hits         7107     2461    -4646     
+ Misses       2508      594    -1914     
Flag Coverage Δ
unittests-Solution 80.55% <90.62%> (?)

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

Files Coverage Δ
...Telemetry.Exporter.InfluxDB/InfluxDBEventSource.cs 60.00% <ø> (ø)
...ry.Exporter.InfluxDB/InfluxDBExporterExtensions.cs 100.00% <100.00%> (ø)
...metry.Exporter.InfluxDB/InfluxDBMetricsExporter.cs 84.61% <ø> (-3.85%) ⬇️
...Telemetry.Exporter.InfluxDB/PointDataExtensions.cs 87.50% <100.00%> (ø)
...ry.Exporter.InfluxDB/TelegrafPrometheusWriterV1.cs 100.00% <ø> (ø)
...ry.Exporter.InfluxDB/TelegrafPrometheusWriterV2.cs 100.00% <ø> (ø)
...stana/Implementation/InstanaExporterEventSource.cs 0.00% <ø> (ø)
...try.Exporter.Instana/Implementation/InstanaSpan.cs 100.00% <ø> (ø)
...orter.Instana/Implementation/InstanaSpanFactory.cs 100.00% <ø> (ø)
...er.Instana/Implementation/InstanaSpanSerializer.cs 93.79% <ø> (ø)
... and 43 more

... and 213 files with indirect coverage changes

@gao-artur gao-artur marked this pull request as ready for review December 19, 2023 07:18
@gao-artur gao-artur requested a review from a team December 19, 2023 07:18
@Kielek Kielek added the comp:instrumentation.hangfire Things related to OpenTelemetry.Instrumentation.Hangfire label Dec 19, 2023
@Kielek
Copy link
Contributor

Kielek commented Dec 19, 2023

@fred2u, please review

@utpilla
Copy link
Contributor

utpilla commented Dec 19, 2023

@gao-artur Did you run into any specific issues when bumping the Microsoft.Extensions.* package versions?

@gao-artur
Copy link
Contributor Author

I have seen that I need to bump too many packages and will have to do the same in all our repositories that use OTel and stopped. From my experience EVERY major version of these packages bring regression bugs and this is not something I'm ready to do as part of OTel upgrade. This should be done as separate story and tested very well.
I explained in the referenced issue why this upgrade is problematic and why I thing OTel team should change their policy of dependencies versioning.

Copy link
Contributor

@utpilla utpilla left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We could have a release based on 1.6.0 but we should upgrade the dependency to 1.7.0 right after the next pre-release version of this package.

@gao-artur
Copy link
Contributor Author

Sure. Will revert immediately after the release.

@Kielek
Copy link
Contributor

Kielek commented Dec 19, 2023

@gao-artur. merging. Please create PR with the release request (just changelog.md). I can handle release tommorow morning my time.

@Kielek Kielek merged commit b9abf08 into open-telemetry:main Dec 19, 2023
30 checks passed
@gao-artur gao-artur mentioned this pull request Dec 20, 2023
3 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
comp:instrumentation.hangfire Things related to OpenTelemetry.Instrumentation.Hangfire
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Release Hangfire instrumentation with OTel v1.6.0?
4 participants