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

Simplify otel-collector example #3560

Conversation

aqaurius6666
Copy link

@aqaurius6666 aqaurius6666 commented Dec 30, 2022

Why

Resolve #2238

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Dec 30, 2022

CLA Signed

The committers listed above are authorized under a signed CLA.

@codecov
Copy link

codecov bot commented Dec 30, 2022

Codecov Report

Merging #3560 (f77b3f9) into main (efd8a7d) will increase coverage by 0.0%.
The diff coverage is n/a.

Additional details and impacted files

Impacted file tree graph

@@          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           
Impacted Files Coverage Δ
exporters/jaeger/jaeger.go 91.1% <0.0%> (+0.8%) ⬆️
sdk/trace/batch_span_processor.go 81.9% <0.0%> (+0.8%) ⬆️

@aqaurius6666 aqaurius6666 changed the title feat: simplify otel-collector example Simplify otel-collector example Dec 31, 2022
CHANGELOG.md Outdated Show resolved Hide resolved
@hanyuancheung hanyuancheung added the Skip Changelog PRs that do not require a CHANGELOG.md entry label Jan 2, 2023
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
Copy link
Member

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.

Copy link
Author

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 👍

// work begins
ctx, span := tracer.Start(
ctx,
"CollectorExporter-Example",
trace.WithAttributes(commonAttrs...))
defer span.End()
meter, _ := metricglobal.Meter("test-meter").SyncInt64().Counter("test-counter")
Copy link
Member

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))
Copy link
Member

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.

@pellared
Copy link
Member

pellared commented Jan 4, 2023

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... 😉

@XSAM
Copy link
Member

XSAM commented May 2, 2024

Closing this, as it has been superseded by #5244

@XSAM XSAM closed this May 2, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Skip Changelog PRs that do not require a CHANGELOG.md entry
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Simplify the otel-collector example
5 participants