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

Allow non-comparable global types #2773

Merged
merged 4 commits into from Apr 7, 2022
Merged

Conversation

MrAlias
Copy link
Contributor

@MrAlias MrAlias commented Apr 7, 2022

The global MeterProvider, TracerProvider, and TextMapPropagator should not panic when they are set to a non-comparable implementation of each.

Fix #2772

The global MeterProvider, TracerProvider, and TextMapPropagator should
not panic when they are set to a non-comparable implementation of each.
@MrAlias MrAlias changed the title Fix #2772: allow non-comparable global types Allow non-comparable global types Apr 7, 2022
@MrAlias MrAlias added this to the Release v1.6.3 milestone Apr 7, 2022
@codecov
Copy link

codecov bot commented Apr 7, 2022

Codecov Report

Merging #2773 (3272707) into main (376c23c) will decrease coverage by 0.0%.
The diff coverage is 100.0%.

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2773     +/-   ##
=======================================
- Coverage   76.8%   76.8%   -0.1%     
=======================================
  Files        181     181             
  Lines      12188   12193      +5     
=======================================
+ Hits        9362    9365      +3     
- Misses      2601    2603      +2     
  Partials     225     225             
Impacted Files Coverage Δ
internal/global/state.go 100.0% <100.0%> (ø)
metric/internal/global/state.go 100.0% <100.0%> (ø)
exporters/jaeger/jaeger.go 90.3% <0.0%> (-0.9%) ⬇️

@dmathieu
Copy link
Member

dmathieu commented Apr 7, 2022

This is fine by me. Just want to reference that the comparison was moved out of the Once following this comment.

@MrAlias
Copy link
Contributor Author

MrAlias commented Apr 7, 2022

This is fine by me. Just want to reference that the comparison was moved out of the Once following this comment.

That still seems like the appropriate behavior for the reasons listed in the linked comment, we don't want to fail the delegation forever if the user sets the delegate to itself. It was correct for that change to have been made. Iterative improvement is the name of the game.

@MrAlias
Copy link
Contributor Author

MrAlias commented Apr 7, 2022

Merging prior to the 1 working day project policy. The goal is to get a patch release out ASAP with this change.

@MrAlias MrAlias merged commit 9838bba into open-telemetry:main Apr 7, 2022
@MrAlias MrAlias deleted the fix-2772 branch April 7, 2022 20:22
@MrAlias MrAlias mentioned this pull request Apr 7, 2022
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.

panic: comparing uncomparable type propagation.compositeTextMapPropagator
5 participants