Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Bug]: Jaeger Agent's http /sampling endpoint returns JSON which breaks older jaeger clients #3891

Closed
vprithvi opened this issue Aug 31, 2022 · 0 comments · Fixed by #3897
Closed
Labels

Comments

@vprithvi
Copy link
Contributor

vprithvi commented Aug 31, 2022

What happened?

Jaeger Agent's sampling endpoint's perOperationStrategies field returns null instead of [], which breaks older Java jaeger-clients.

Steps to reproduce

As of 1.23, sampling endpoint returns the following when using grpc as transport between agent and collector.

$ curl "localhost:5778/sampling?service=none" | jq
 
  "strategyType": "PROBABILISTIC",
  "probabilisticSampling": {
    "samplingRate": 0
  },
  "operationSampling": {
    "defaultSamplingProbability": 0.001,
    "defaultLowerBoundTracesPerSecond": 1e-05,
    "perOperationStrategies": null
  }
}

Expected behavior

As of version 1.17, sampling endpoint returns the following when using tchannel as transport between agent and collector

$ curl "localhost:5778/sampling?service=none" | jq
 
  "strategyType": "PROBABILISTIC",
  "probabilisticSampling": {
    "samplingRate": 0
  },
  "operationSampling": {
    "defaultSamplingProbability": 0.001,
    "defaultLowerBoundTracesPerSecond": 1e-05,
    "perOperationStrategies": []
  }
}

This is because the grpc/proto response from collector uses nil as the default value for slice, and json.Marshal then encodes nil as null. It is possible that tchannel/thrift didn't follow the same conventions and used empty arrays instead.

From https://pkg.go.dev/encoding/json#Marshal,

Array and slice values encode as JSON arrays, except that []byte encodes as a base64-encoded string, and a nil slice encodes as the null JSON value.

@vprithvi vprithvi added the bug label Aug 31, 2022
vprithvi added a commit to vprithvi/jaeger that referenced this issue Aug 31, 2022
…`[]`

- Returning an empty slice for `perOperationStrategies` instead of the default nil slice allows
  json.Marshal to marshal it to `[]` instead of `null`
- Fixes jaegertracing#3891

Signed-off-by: Prithvi Raj <[email protected]>
yurishkuro pushed a commit that referenced this issue Sep 1, 2022
…ON (#3897)

* fix: jaeger-agent sampling endpoint `perOperationStrategies` returns `[]`

- Returning an empty slice for `perOperationStrategies` instead of the default nil slice allows
  json.Marshal to marshal it to `[]` instead of `null`
- Fixes #3891

Signed-off-by: Prithvi Raj <[email protected]>

* fmt

Signed-off-by: Prithvi Raj <[email protected]>

* simplify

Signed-off-by: Prithvi Raj <[email protected]>

Signed-off-by: Prithvi Raj <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
1 participant