-
Notifications
You must be signed in to change notification settings - Fork 2.5k
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
[WIP] OTEL / Prom metrics benchmark #5676
Draft
yurishkuro
wants to merge
23
commits into
jaegertracing:main
Choose a base branch
from
yurishkuro:otel_metrics_benchmark
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Draft
Changes from all commits
Commits
Show all changes
23 commits
Select commit
Hold shift + click to select a range
f1f9b7a
Instantiated OTEL Metrics
Wise-Wizard 12403b1
Ran make fmt
Wise-Wizard 175efa3
Merge branch 'main' into OTEL_Metrics
Wise-Wizard 443858f
Merge branch 'main' into OTEL_Metrics
Wise-Wizard 0a073b6
Create otelmetrics package
Wise-Wizard aa92107
Merge branch 'main' into OTEL_Metrics
Wise-Wizard 7efd9b9
Update pkg/metrics/otelmetrics/factory.go
Wise-Wizard a70d64a
Created benchmark to compare metric implementation
Wise-Wizard 2ab543b
Merge branch 'OTEL_Metrics' of https://github.com/Wise-Wizard/jaeger …
Wise-Wizard dc2a9ed
Resolved conflicts
Wise-Wizard f508b8a
Merge branch 'main' into OTEL_Metrics
Wise-Wizard 4a99291
avoid allocations
yurishkuro 099ba05
Implemented changes
Wise-Wizard 21c14b4
Merge branch 'main' into OTEL_Metrics
Wise-Wizard 8b47b12
Used metrics.NullCounter as return type
Wise-Wizard 8032ce2
Created rough implementation of OTEL SDK
Wise-Wizard 6ec5ec4
Fixed initialization errors
Wise-Wizard 33d5855
Merge branch 'main' into OTEL_Metrics
Wise-Wizard 4e7dcbb
Made suggested changes
Wise-Wizard f29cad5
Created Prometheus Exporter
Wise-Wizard 723d74a
OTEL / Prom metrics benchmark
yurishkuro c8735f6
add-census
yurishkuro d04050c
more
yurishkuro File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,131 @@ | ||
// Copyright (c) 2024 The Jaeger Authors. | ||
// SPDX-License-Identifier: Apache-2.0 | ||
|
||
package benchmark_test | ||
|
||
import ( | ||
"context" | ||
"testing" | ||
|
||
promsdk "github.com/prometheus/client_golang/prometheus" | ||
"github.com/stretchr/testify/require" | ||
"go.opentelemetry.io/otel/attribute" | ||
"go.opentelemetry.io/otel/bridge/opencensus" | ||
"go.opentelemetry.io/otel/metric" | ||
sdkmetric "go.opentelemetry.io/otel/sdk/metric" | ||
|
||
promExporter "go.opentelemetry.io/otel/exporters/prometheus" | ||
) | ||
|
||
func BenchmarkVarags(b *testing.B) { | ||
f := func(opts ...string) int { | ||
return len(opts) | ||
} | ||
b.ResetTimer() | ||
for i := 0; i < b.N; i++ { | ||
f("x", "y", "z") | ||
} | ||
} | ||
|
||
func BenchmarkAddConfig(b *testing.B) { | ||
attrSet := attribute.NewSet(attribute.String("tag1", "value1")) | ||
attrOpt := metric.WithAttributeSet(attrSet) | ||
f := func(opts ...metric.AddOption) int { | ||
metric.NewAddConfig(opts) | ||
return len(opts) | ||
} | ||
b.ResetTimer() | ||
for i := 0; i < b.N; i++ { | ||
f(attrOpt) | ||
} | ||
} | ||
|
||
func BenchmarkPrometheusCounter(b *testing.B) { | ||
reg := promsdk.NewRegistry() | ||
opts := promsdk.CounterOpts{ | ||
Name: "test_counter", | ||
Help: "help", | ||
} | ||
cv := promsdk.NewCounterVec(opts, []string{"tag1"}) | ||
reg.MustRegister(cv) | ||
counter := cv.WithLabelValues("value1") | ||
|
||
b.ResetTimer() | ||
for i := 0; i < b.N; i++ { | ||
counter.Add(1) | ||
} | ||
} | ||
|
||
func BenchmarkOTELCounter(b *testing.B) { | ||
counter := otelCounter(b) | ||
ctx := context.Background() | ||
|
||
b.ResetTimer() | ||
for i := 0; i < b.N; i++ { | ||
counter.Add(ctx, 1) | ||
} | ||
} | ||
|
||
func BenchmarkCencusCounter(b *testing.B) { | ||
counter := censusCounter(b) | ||
ctx := context.Background() | ||
|
||
b.ResetTimer() | ||
for i := 0; i < b.N; i++ { | ||
counter.Add(ctx, 1) | ||
} | ||
} | ||
|
||
func BenchmarkOTELCounterWithLabel(b *testing.B) { | ||
counter := otelCounter(b) | ||
ctx := context.Background() | ||
attrSet := attribute.NewSet(attribute.String("tag1", "value1")) | ||
attrOpt := metric.WithAttributeSet(attrSet) | ||
|
||
b.ResetTimer() | ||
for i := 0; i < b.N; i++ { | ||
counter.Add(ctx, 1, attrOpt) | ||
} | ||
} | ||
func BenchmarkCencusCounterWithLabel(b *testing.B) { | ||
counter := censusCounter(b) | ||
ctx := context.Background() | ||
attrSet := attribute.NewSet(attribute.String("tag1", "value1")) | ||
attrOpt := metric.WithAttributeSet(attrSet) | ||
|
||
b.ResetTimer() | ||
for i := 0; i < b.N; i++ { | ||
counter.Add(ctx, 1, attrOpt) | ||
} | ||
} | ||
|
||
func otelCounter(b *testing.B) metric.Int64Counter { | ||
registry := promsdk.NewRegistry() | ||
exporter, err := promExporter.New(promExporter.WithRegisterer(registry)) | ||
require.NoError(b, err) | ||
meterProvider := sdkmetric.NewMeterProvider( | ||
sdkmetric.WithReader(exporter), | ||
) | ||
|
||
meter := meterProvider.Meter("test") | ||
counter, err := meter.Int64Counter("test_counter") | ||
require.NoError(b, err) | ||
return counter | ||
} | ||
|
||
func censusCounter(b *testing.B) metric.Int64Counter { | ||
registry := promsdk.NewRegistry() | ||
exporter, err := promExporter.New( | ||
promExporter.WithRegisterer(registry), | ||
promExporter.WithProducer(opencensus.NewMetricProducer()), | ||
) | ||
require.NoError(b, err) | ||
meterProvider := sdkmetric.NewMeterProvider( | ||
sdkmetric.WithReader(exporter), | ||
) | ||
|
||
meter := meterProvider.Meter("test") | ||
counter, err := meter.Int64Counter("test_counter") | ||
require.NoError(b, err) | ||
return counter | ||
} |
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Curious how you feel about these two lines, and why you've excluded them from the benchmark? Ergonomically speaking, would you cache the
attrOpt
value after computing it? If so, you'd be better off with bound instruments. If not, you should measure it. I ask because the introduction of functional options adds a bunch of allocations, so unless you compute and re-use the []Option slice, you've either got an ergonomics problem or a performance problem. We stopped using the OTel-Go API as a result of this and installed a more-efficient functional-option-free bypass. lightstep/otel-launcher-go#446There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jmacd yes, that's exactly what Jaeger is doing, we always used "bound" instruments.
jaeger/internal/metrics/otelmetrics/counter.go
Lines 12 to 20 in 8a33800
But this does not help to completely avoid allocations. It's not the passing of vararg options that's causing the allocations, it's something deeper in the implementation.