[extension/jaegerremotesampling] This extension may be broken after importing Jaeger v1.42 code

[yurishkuro]

Component(s)

extension/jaegerremotesampling

Describe the issue you're reporting

This is just a heads-up issue with some pointers.

This extension uses json.Marshal from stdlib to render the sampling strategy:

opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/http.go

Line 117 in 8049c8c

jsonBytes, err := json.Marshal(resp)

and has a unit test that validates the format (kudos):

opentelemetry-collector-contrib/extension/jaegerremotesampling/internal/http_test.go

Line 90 in 8049c8c

assert.Equal(t, `{"strategyType":"PROBABILISTIC"}`, body)

However, it passes an auto-inferred type from samplingStore to json.Marshal, which happens to be Thrift-gen struct. This is changing in the next Jaeger release (jaegertracing/jaeger#4181) to return Protobuf-gen struct, that may not work with stdlib json.Marshal (in Jaeger we're using jsonpb.Marshal from gogoproto).

Related issues:

• [samplers/jaegerremote] Invalid expectation of JSON format opentelemetry-go-contrib#3184
• Invalid expectations of JSON format for Jaeger remote sampling strategies opentelemetry-specification#3126

Also, this extension attempts to emulate Jaeger agent endpoints, but their behavior on / and /sampling endpoint is actually different (explained in open-telemetry/opentelemetry-go-contrib#3184)

[github-actions]

Pinging code owners:

• extension/jaegerremotesampling: @jpkrohling @frzifus

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[jpkrohling]

@frzifus, are you able to handle this one?

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• extension/jaegerremotesampling: @jpkrohling @frzifus

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[jpkrohling]

ping @frzifus

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• extension/jaegerremotesampling: @jpkrohling @frzifus

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

[github-actions]

This issue has been closed as inactive because it has been stale for 120 days with no activity.