Skip to content

Add the experimental exemplar feature#4871

Merged
MrAlias merged 13 commits into
open-telemetry:mainfrom
MrAlias:add-x-exemplars
Jan 31, 2024
Merged

Add the experimental exemplar feature#4871
MrAlias merged 13 commits into
open-telemetry:mainfrom
MrAlias:add-x-exemplars

Conversation

@MrAlias
Copy link
Copy Markdown
Contributor

@MrAlias MrAlias commented Jan 30, 2024

Integrate the exemplar package into the metric SDK as an experimental feature.

Closes #4455
Resolves #559

Benchmark

$ 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

@codecov
Copy link
Copy Markdown

codecov Bot commented Jan 30, 2024

Codecov Report

Attention: 4 lines in your changes are missing coverage. Please review.

Comparison is base (d9d9507) 82.5% compared to head (771557c) 82.6%.

Additional details and impacted files

Impacted file tree graph

@@           Coverage Diff           @@
##            main   #4871     +/-   ##
=======================================
+ Coverage   82.5%   82.6%   +0.1%     
=======================================
  Files        231     232      +1     
  Lines      18762   18872    +110     
=======================================
+ Hits       15488   15600    +112     
+ Misses      2985    2982      -3     
- Partials     289     290      +1     
Files Coverage Δ
sdk/metric/internal/aggregate/aggregate.go 100.0% <100.0%> (ø)
...metric/internal/aggregate/exponential_histogram.go 96.3% <100.0%> (+0.1%) ⬆️
sdk/metric/internal/aggregate/histogram.go 100.0% <100.0%> (ø)
sdk/metric/internal/aggregate/lastvalue.go 100.0% <100.0%> (ø)
sdk/metric/internal/aggregate/sum.go 100.0% <100.0%> (ø)
sdk/metric/pipeline.go 87.5% <100.0%> (+1.0%) ⬆️
sdk/metric/exemplar.go 92.5% <92.5%> (ø)

... and 1 file with indirect coverage changes

@MrAlias MrAlias added the area:metrics Part of OpenTelemetry Metrics label Jan 30, 2024
@MrAlias MrAlias added this to the v1.23.0 milestone Jan 30, 2024
@MrAlias MrAlias marked this pull request as ready for review January 30, 2024 16:39
Comment thread sdk/metric/internal/aggregate/exponential_histogram.go Outdated
Comment thread sdk/metric/exemplar.go Outdated
Copy link
Copy Markdown
Collaborator

@dashpole dashpole left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

implementation looks good, but it looks like it needs unit tests?

@MrAlias
Copy link
Copy Markdown
Contributor Author

MrAlias commented Jan 30, 2024

implementation looks good, but it looks like it needs unit tests?

Right. Added c589d01

PTAL

Comment thread sdk/metric/EXPERIMENTAL.md
Comment thread sdk/metric/benchmark_test.go
Comment thread sdk/metric/benchmark_test.go Outdated
Comment thread sdk/metric/benchmark_test.go
Comment thread sdk/metric/EXPERIMENTAL.md
Comment thread sdk/metric/pipeline_test.go
Comment thread sdk/metric/pipeline_test.go
Default and invalid OTEL_METRICS_EXEMPLAR_FILTER.

Test sampled and non-sampled context for trace_based.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:metrics Part of OpenTelemetry Metrics

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Add support for exemplars

3 participants