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

feat: Add OpenTelemetry setup function #11

Closed
wants to merge 1 commit into from
Closed

Conversation

mzaniolo
Copy link

@mzaniolo mzaniolo commented Jan 6, 2025

This PR adds a function to setup the Opentelemetry exporting for logs, traces and metrics.
This function will setup everything, all that will be left to do on the services is to setup the propagation layer (example for axum) and configure the #[instrument] macro on the functions.

For metric I setup to use the traces for metrics using the crate tracing-opentelemetry. Other options are the opentelemetry sdk itself, and the metrics.

The metrics doesn't seem to have an opentelemtry exporter but it does have an prometheus exporter. If we choose to use an collector that supports both, otel and prometheus endpoints, like grafana alloy, the metrics crate can also be an option

@mzaniolo mzaniolo requested a review from a team as a code owner January 6, 2025 11:39
@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch from 7eb6574 to 181e5cf Compare January 6, 2025 14:18
Copy link

codecov bot commented Jan 6, 2025

Codecov Report

Attention: Patch coverage is 86.10039% with 36 lines in your changes missing coverage. Please review.

Project coverage is 77.80%. Comparing base (77b970a) to head (b922cd1).

✅ All tests successful. No failed tests found.

Files with missing lines Patch % Lines
src/telemetry/mod.rs 87.28% 29 Missing ⚠️
src/telemetry/reqwest_middleware.rs 0.00% 7 Missing ⚠️
Additional details and impacted files
@@             Coverage Diff             @@
##             main      #11       +/-   ##
===========================================
+ Coverage   67.24%   77.80%   +10.55%     
===========================================
  Files           5        8        +3     
  Lines         232      491      +259     
===========================================
+ Hits          156      382      +226     
- Misses         76      109       +33     
Files with missing lines Coverage Δ
src/lib.rs 78.08% <ø> (ø)
src/telemetry/config.rs 100.00% <100.00%> (ø)
src/telemetry/reqwest_middleware.rs 0.00% <0.00%> (ø)
src/telemetry/mod.rs 87.28% <87.28%> (ø)

... and 1 file with indirect coverage changes


Continue to review full report in Codecov by Sentry.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 77b970a...b922cd1. Read the comment docs.

@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from 2cc3d5e to 9f45534 Compare January 7, 2025 15:53
@sirewix
Copy link
Contributor

sirewix commented Jan 8, 2025

We should probably guard this behind cargo feature

src/telemetry.rs Outdated Show resolved Hide resolved
@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 2 times, most recently from 3bb5329 to 4c1d1df Compare January 8, 2025 13:04
src/telemetry/config.rs Outdated Show resolved Hide resolved
@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from 7e9a05f to ac11152 Compare January 8, 2025 17:48
@mzaniolo mzaniolo requested a review from sirewix January 9, 2025 11:19
@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 2 times, most recently from 06f3601 to 5ad809d Compare January 9, 2025 12:23
src/telemetry/config.rs Outdated Show resolved Hide resolved
@sirewix
Copy link
Contributor

sirewix commented Jan 9, 2025

What's the difference between stdout layer and logs layer?

@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch from 5ad809d to e355992 Compare January 9, 2025 18:31
@mzaniolo
Copy link
Author

mzaniolo commented Jan 9, 2025

What's the difference between stdout layer and logs layer?

the stdout layer only logs to the stdout and the logs layer only exports the logs to and otel collector

@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from ea95e16 to 85d0e49 Compare January 10, 2025 13:15
@mzaniolo mzaniolo changed the title WIP: feat: Add OpenTelemetry setup function feat: Add OpenTelemetry setup function Jan 10, 2025
@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from f8994cd to c7bf78c Compare January 10, 2025 13:52
@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from 9f8bb11 to 31bd81d Compare January 10, 2025 17:33
@sirewix sirewix mentioned this pull request Jan 10, 2025
@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 2 times, most recently from c899ea2 to 4dce311 Compare January 13, 2025 11:30
@mzaniolo mzaniolo linked an issue Jan 13, 2025 that may be closed by this pull request
@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from ea2004a to cc1cd81 Compare January 13, 2025 12:22
src/telemetry/config.rs Outdated Show resolved Hide resolved
src/telemetry/config.rs Outdated Show resolved Hide resolved
@mzaniolo mzaniolo force-pushed the mzaniolo/telemetry branch 4 times, most recently from 5f81383 to f21c2ab Compare January 14, 2025 13:22
Copy link
Contributor

@sirewix sirewix left a comment

Choose a reason for hiding this comment

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

LGTM aside from few cosmetic nits

README.md Outdated Show resolved Hide resolved
src/telemetry/config.rs Outdated Show resolved Hide resolved
src/telemetry/mod.rs Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
README.md Outdated Show resolved Hide resolved
sirewix
sirewix previously approved these changes Jan 16, 2025
@mzaniolo
Copy link
Author

This code got to big and was moved to a separated repo. This PR was replaced by https://github.com/famedly/rust-telemetry/pull/1

@mzaniolo mzaniolo closed this Jan 17, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Add a "base" config
2 participants