[samplers/jaegerremote] Change parser to support enums as both strings and numbers #3183
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
yurishkuro
requested review from
jmacd,
MrAlias,
Aneurysm9,
evantorrie,
XSAM,
dashpole,
MadVikingGod,
pellared,
hanyuancheung and
dmathieu
as code owners
yurishkuro
changed the title
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3183 +/- ##
=====================================
Coverage 69.5% 69.5%
=====================================
Files 147 147
Lines 6890 6894 +4
=====================================
+ Hits 4789 4796 +7
+ Misses 1980 1978 -2
+ Partials 121 120 -1
|
yurishkuro
changed the title
yurishkuro
force-pushed
the
jaegerremote_unittests
branch
from
753305f
to
59b77e2
Compare
Aneurysm9
approved these changes
Jan 27, 2023
yurishkuro
force-pushed
the
jaegerremote_unittests
branch
from
2b2fa87
to
bf203f1
Compare
MrAlias
approved these changes
Jan 27, 2023
|
can this be merged? |
hanyuancheung
approved these changes
Feb 8, 2023
|
Hi, Can you please provide the release plan for this changes? |
rfratto
added a commit
to rfratto/agent
that referenced
this pull request
Mar 11, 2023
This fixes an issue where the remote sampler could not be parsed if it used strings for the strategy type instead of numbers. The resolution is to switch to the protobuf JSON encoding, which properly parses this type as a string. Not being able to parse the remote sampler rules meant that sampling rate fell back to the default (10%). This change is equivalent to open-telemetry/opentelemetry-go-contrib#3183. We are still unable to move to the upstream type while open-telemetry/opentelemetry-go-contrib#2981 remains unresolved.
rfratto
added a commit
to grafana/agent
that referenced
this pull request
Mar 11, 2023
This fixes an issue where the remote sampler could not be parsed if it used strings for the strategy type instead of numbers. The resolution is to switch to the protobuf JSON encoding, which properly parses this type as a string. Not being able to parse the remote sampler rules meant that sampling rate fell back to the default (10%). This change is equivalent to open-telemetry/opentelemetry-go-contrib#3183. We are still unable to move to the upstream type while open-telemetry/opentelemetry-go-contrib#2981 remains unresolved.
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Resolves #3184
encoding/jsontogogoproto/jsonpbparser that works for both string and number enums in JSON