Add exemplars to the metric SDK as an experimental feature #4455
Conversation
MrAlias
added
pkg:SDK
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## main #4455 +/- ##
=======================================
- Coverage 82.3% 82.3% -0.1%
=======================================
Files 226 232 +6
Lines 18481 18710 +229
=======================================
+ Hits 15224 15400 +176
- Misses 2971 3023 +52
- Partials 286 287 +1
|
sdk/metric/exemplar.go
Outdated
|
|
||
| // TODO: This is not defined by the specification, nor is the mechanism to | ||
| // configure it. | ||
| const defaultFixedSize = 1 |
There was a problem hiding this comment.
FYI - I'm planning to update the specification here witha MAY. 1 is the fallback default, but we found in Java that a simple optimisation is to set this = the number of available CPUs for the runtime (if that's easy to grab). It can dramatically reduce contention on writing exemplars in extreme cases for not a huge investment of memory.
I'm not sure if Go suffers from the same contention issues, but might be worth benchmarking.
There was a problem hiding this comment.
Ah, that's interesting. I need to look into it.
My guess is this would also benefit in using that value (which is pretty accessible).
sdk/metric/exemplar.go
Outdated
| } | ||
|
|
||
| // https://github.com/open-telemetry/opentelemetry-specification/blob/d4b241f451674e8f611bb589477680341006ad2b/specification/configuration/sdk-environment-variables.md#exemplar | ||
| const filterEnvKey = "OTEL_METRICS_EXEMPLAR_FILTER" |
There was a problem hiding this comment.
FYI - #2421 - I think we're going to recommend that the filter is a configuration setting on the SDK (likely meter provider).
IIUC the architecture, this would still be feasible in Go?
There was a problem hiding this comment.
That makes sense to me, but we will have to wait for exemplars to become stable to add it to the stable API interface of the SDK.
I was talking with @dashpole and I was also wondering if we even need exemplar filters to start here. Might be something to consider at the spec level.
There was a problem hiding this comment.
open-telemetry/opentelemetry-specification#3820 <- The interface no longer needs to be exposed to users, but the configuration needs to be SDK-wide.
This comment was marked as outdated.
This comment was marked as outdated.
|
Waiting on #3011. That change will influence the design of the exemplar reservoir interface. If it is accepted, the current design should be used. But, if it is not implemented, there is an optimization to be made given the dropped attributes are calculated during each measurement. It is anticipated to be accepted. |
|
It looks like recording dropped attributes during $ benchstat old.txt new.txt
goos: linux
goarch: amd64
pkg: go.opentelemetry.io/otel/sdk/metric
cpu: Intel(R) Core(TM) i7-8550U CPU @ 1.80GHz
│ old.txt │ new.txt │
│ sec/op │ sec/op vs base │
Exemplars/Int64Counter/8-8 15.20µ ± 2% 13.87µ ± 4% -8.76% (p=0.000 n=10)
Exemplars/Int64Histogram/8-8 12.35µ ± 3% 11.90µ ± 1% -3.66% (p=0.000 n=10)
geomean 13.70µ 12.85µ -6.24%
│ old.txt │ new.txt │
│ B/op │ B/op vs base │
Exemplars/Int64Counter/8-8 3.789Ki ± 0% 3.789Ki ± 0% ~ (p=1.000 n=10) ¹
Exemplars/Int64Histogram/8-8 3.875Ki ± 0% 3.875Ki ± 0% ~ (p=1.000 n=10) ¹
geomean 3.832Ki 3.832Ki +0.00%
¹ all samples are equal
│ old.txt │ new.txt │
│ allocs/op │ allocs/op vs base │
Exemplars/Int64Counter/8-8 84.00 ± 0% 84.00 ± 0% ~ (p=1.000 n=10) ¹
Exemplars/Int64Histogram/8-8 72.00 ± 0% 72.00 ± 0% ~ (p=1.000 n=10) ¹
geomean 77.77 77.77 +0.00%
¹ all samples are equal |
No description provided.