diff --git a/CHANGELOG.md b/CHANGELOG.md index 540bc79fa4f..aa13e563ed8 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -12,12 +12,17 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm - Add the new `go.opentelemetry.io/contrib/instrgen` package to provide auto-generated source code instrumentation. (#3068) +### Changed + +- `samplers/jaegerremote`: change to use protobuf parser instead of encoding/json to accept enums as strings. (#3183) + ## [1.14.0/0.39.0/0.8.0] - 2023-02-07 ### Changed - Change `runtime.uptime` instrument in `go.opentelemetry.io/contrib/instrumentation/runtime` from `Int64ObservableUpDownCounter` to `Int64ObservableCounter`, since the value is monotonic. (#3347) +- `samplers/jaegerremote`: change to use protobuf parser instead of encoding/json to accept enums as strings. (#3183) ### Fixed diff --git a/samplers/jaegerremote/README.md b/samplers/jaegerremote/README.md index 249a91b79d1..fa2de2a0f84 100644 --- a/samplers/jaegerremote/README.md +++ b/samplers/jaegerremote/README.md @@ -1,6 +1,6 @@ # Jaeger Remote Sampler -This package implements [Jaeger remote sampler](https://www.jaegertracing.io/docs/latest/sampling/#collector-sampling-configuration). +This package implements [Jaeger remote sampler](https://www.jaegertracing.io/docs/latest/sampling/#remote-sampling). Remote sampler allows defining sampling configuration for services at the backend, at the granularity of service + endpoint. When using the Jaeger backend, the sampling configuration can come from two sources: diff --git a/samplers/jaegerremote/sampler_remote.go b/samplers/jaegerremote/sampler_remote.go index c6aed231125..6deb29427ac 100644 --- a/samplers/jaegerremote/sampler_remote.go +++ b/samplers/jaegerremote/sampler_remote.go @@ -17,7 +17,7 @@ package jaegerremote // import "go.opentelemetry.io/contrib/samplers/jaegerremote" import ( - "encoding/json" + "bytes" "fmt" "io" "net/http" @@ -26,6 +26,8 @@ import ( "sync/atomic" "time" + "github.com/gogo/protobuf/jsonpb" + jaeger_api_v2 "go.opentelemetry.io/contrib/samplers/jaegerremote/internal/proto-gen/jaeger-idl/proto/api_v2" "go.opentelemetry.io/otel/sdk/trace" ) @@ -311,7 +313,11 @@ type samplingStrategyParserImpl struct{} func (p *samplingStrategyParserImpl) Parse(response []byte) (interface{}, error) { strategy := new(jaeger_api_v2.SamplingStrategyResponse) - if err := json.Unmarshal(response, strategy); err != nil { + // Official Jaeger Remote Sampling protocol contains enums encoded as strings. + // Legacy protocol contains enums as numbers. + // Gogo's jsonpb module can parse either format. + // Cf. https://github.com/open-telemetry/opentelemetry-go-contrib/issues/3184 + if err := jsonpb.Unmarshal(bytes.NewReader(response), strategy); err != nil { return nil, err } return strategy, nil diff --git a/samplers/jaegerremote/sampler_remote_test.go b/samplers/jaegerremote/sampler_remote_test.go index 7748b751f40..322400a9c72 100644 --- a/samplers/jaegerremote/sampler_remote_test.go +++ b/samplers/jaegerremote/sampler_remote_test.go @@ -535,3 +535,55 @@ func getSamplingStrategyResponse(strategyType jaeger_api_v2.SamplingStrategyType } return nil } + +func TestSamplingStrategyParserImpl(t *testing.T) { + assertProbabilistic := func(t *testing.T, s *jaeger_api_v2.SamplingStrategyResponse) { + require.NotNil(t, s.GetProbabilisticSampling(), "output: %+v", s) + require.EqualValues(t, 0.42, s.GetProbabilisticSampling().GetSamplingRate(), "output: %+v", s) + } + assertRateLimiting := func(t *testing.T, s *jaeger_api_v2.SamplingStrategyResponse) { + require.NotNil(t, s.GetRateLimitingSampling(), "output: %+v", s) + require.EqualValues(t, 42, s.GetRateLimitingSampling().GetMaxTracesPerSecond(), "output: %+v", s) + } + tests := []struct { + name string + json string + assert func(t *testing.T, s *jaeger_api_v2.SamplingStrategyResponse) + }{ + { + name: "official JSON probabilistic", + json: `{"strategyType":"PROBABILISTIC","probabilisticSampling":{"samplingRate":0.42}}`, + assert: assertProbabilistic, + }, + { + name: "official JSON rate limiting", + json: `{"strategyType":"RATE_LIMITING","rateLimitingSampling":{"maxTracesPerSecond":42}}`, + assert: assertRateLimiting, + }, + { + name: "legacy JSON probabilistic", + json: `{"strategyType":0,"probabilisticSampling":{"samplingRate":0.42}}`, + assert: assertProbabilistic, + }, + { + name: "legacy JSON rate limiting", + json: `{"strategyType":1,"rateLimitingSampling":{"maxTracesPerSecond":42}}`, + assert: assertRateLimiting, + }, + } + for _, test := range tests { + t.Run(test.name, func(t *testing.T) { + val, err := new(samplingStrategyParserImpl).Parse([]byte(test.json)) + require.NoError(t, err) + s := val.(*jaeger_api_v2.SamplingStrategyResponse) + test.assert(t, s) + }) + } +} + +func TestSamplingStrategyParserImpl_Error(t *testing.T) { + json := `{"strategyType":"foo_bar","probabilisticSampling":{"samplingRate":0.42}}` + val, err := new(samplingStrategyParserImpl).Parse([]byte(json)) + require.Error(t, err, "output: %+v", val) + require.Contains(t, err.Error(), `unknown value "foo_bar"`) +}