-
Notifications
You must be signed in to change notification settings - Fork 1.1k
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
Simplify otel-collector
example
#3560
Simplify otel-collector
example
#3560
Conversation
Codecov Report
Additional details and impacted files@@ Coverage Diff @@
## main #3560 +/- ##
=====================================
Coverage 77.9% 77.9%
=====================================
Files 163 163
Lines 11850 11850
=====================================
+ Hits 9235 9239 +4
+ Misses 2417 2413 -4
Partials 198 198
|
eb225e9
to
b927198
Compare
b927198
to
c0afbe9
Compare
otel-collector
example
indicating that the Jaeger UI is available at | ||
[http://localhost:80](http://localhost:80). Navigate there in your favorite | ||
Jaeger UI is available at | ||
[http://localhost:16686](http://localhost:16686). Navigate there in your favorite |
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.
For Jaeger and Prometheus, how about giving a couple indications on the generated data, and what to look for rather than only the link to the interfaces?
Jaeger could mention to look into test-service
go view the generated trace. Prometheus could give a sample query.
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.
@dmathieu I've updated as your suggestion 👍
example/otel-collector/main.go
Outdated
// work begins | ||
ctx, span := tracer.Start( | ||
ctx, | ||
"CollectorExporter-Example", | ||
trace.WithAttributes(commonAttrs...)) | ||
defer span.End() | ||
meter, _ := metricglobal.Meter("test-meter").SyncInt64().Counter("test-counter") |
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.
How about making this look less dummy than test-counter
? This is counting iterations. So it could be named loop-counter
?
// Shutdown will flush any remaining spans and shut down the exporter. | ||
return tracerProvider.Shutdown, nil | ||
// Set up a metric exporter | ||
metricExporter, err := otlpmetricgrpc.New(ctx, otlpmetricgrpc.WithGRPCConn(conn)) |
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.
For readability, I would split setup into 2 methods. One for tracing and one for metrics.
I was working on something similar recently for .NET and here is the result: https://github.com/open-telemetry/opentelemetry-dotnet-instrumentation/tree/main/examples/demo Maybe it will help... 😉 |
Closing this, as it has been superseded by #5244 |
Why
Resolve #2238