opentelemetry-go-contrib icon indicating copy to clipboard operation
opentelemetry-go-contrib copied to clipboard

fix jaegerremote parser to use jsonpb unmarshaler

Open tonybase opened this issue 1 year ago • 4 comments

Failed to parse sampling strategy response: json: cannot unmarshal string into Go struct field SamplingStrategyResponse.strategyType of type api_v2.SamplingStrategyType:

{"strategyType":"PROBABILISTIC","probabilisticSampling":{"samplingRate":1},"operationSampling":{"defaultSamplingProbability":1,"defaultLowerBoundTracesPerSecond":0,"perOperationStrategies":[{"operation":"/health","probabilisticSampling":{"samplingRate":0}},{"operation":"/metrics","probabilisticSampling":{"samplingRate":0}}}],"defaultUpperBoundTracesPerSecond":0}}

tonybase avatar Aug 05 '22 12:08 tonybase

CLA Signed

The committers listed above are authorized under a signed CLA.

  • :white_check_mark: login: tonybase / name: Tony Chen (d02758470adf5cd1750d3fc1f61f8cd679fd9441)

Codecov Report

Merging #2623 (944e1b4) into main (3091f5e) will increase coverage by 5.0%. The diff coverage is 0.0%.

:exclamation: Current head 944e1b4 differs from pull request most recent head 9b204a7. Consider uploading reports for the commit 9b204a7 to get more accurate results

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #2623     +/-   ##
=======================================
+ Coverage   69.4%   74.4%   +5.0%     
=======================================
  Files        145     145             
  Lines       6707    6708      +1     
=======================================
+ Hits        4655    4993    +338     
+ Misses      1938    1567    -371     
- Partials     114     148     +34     
Impacted Files Coverage Δ
samplers/jaegerremote/sampler_remote.go 86.8% <0.0%> (-0.6%) :arrow_down:
instrumentation/net/http/otelhttp/config.go 87.8% <0.0%> (+7.5%) :arrow_up:
instrumentation/runtime/runtime.go 74.0% <0.0%> (+74.0%) :arrow_up:
instrumentation/host/host.go 74.4% <0.0%> (+74.4%) :arrow_up:
instrumentation/host/version.go 100.0% <0.0%> (+100.0%) :arrow_up:
instrumentation/runtime/version.go 100.0% <0.0%> (+100.0%) :arrow_up:

codecov[bot] avatar Aug 07 '22 15:08 codecov[bot]

This PR should implement in your local-copy version, instead of being implemented in contrib. If you use jsonpb to Marshal, you should also copy and reimplement the jsonpb Unmarshal. But I don't think this is the standard way according to the otel specification. PTAL.

hanyuancheung avatar Aug 15 '22 14:08 hanyuancheung

This should include an automated test

dmathieu avatar Aug 15 '22 14:08 dmathieu