Skip to content

Commit

Permalink
squash
Browse files Browse the repository at this point in the history
  • Loading branch information
yurishkuro committed Jan 27, 2023
1 parent fe23c1f commit bf203f1
Show file tree
Hide file tree
Showing 5 changed files with 65 additions and 4 deletions.
4 changes: 3 additions & 1 deletion .gitignore
Original file line number Diff line number Diff line change
Expand Up @@ -4,10 +4,12 @@ Thumbs.db
.tools/
.idea/
.vscode/
go.work
go.work.sum
*.iml
*.so
coverage.*
example

instrumentation/google.golang.org/grpc/otelgrpc/example/server/server
instrumentation/google.golang.org/grpc/otelgrpc/example/client/client
instrumentation/google.golang.org/grpc/otelgrpc/example/client/client
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ This project adheres to [Semantic Versioning](https://semver.org/spec/v2.0.0.htm
### Changed

- `otelgrpc`: Remove expensive calculation of uncompressed message size attribute. (#3168)
- `samplers/jaegerremote`: change to use protobuf parser instead of encoding/json to accept enums as strings. (#3183)

## [1.12.0/0.37.0/0.6.0]

Expand Down
2 changes: 1 addition & 1 deletion samplers/jaegerremote/README.md
Original file line number Diff line number Diff line change
@@ -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:

Expand Down
10 changes: 8 additions & 2 deletions samplers/jaegerremote/sampler_remote.go
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,7 @@
package jaegerremote // import "go.opentelemetry.io/contrib/samplers/jaegerremote"

import (
"encoding/json"
"bytes"
"fmt"
"io"
"net/http"
Expand All @@ -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"
)
Expand Down Expand Up @@ -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
Expand Down
52 changes: 52 additions & 0 deletions samplers/jaegerremote/sampler_remote_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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}}`
any, err := new(samplingStrategyParserImpl).Parse([]byte(json))
require.Error(t, err, "output: %+v", any)
require.Contains(t, err.Error(), `unknown value "foo_bar"`)
}

0 comments on commit bf203f1

Please sign in to comment.