feat: Add OpenTelemetry setup function #11

mzaniolo · 2025-01-06T11:39:36Z

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

codecov · 2025-01-06T14:22:40Z

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 (f21c2ab).

✅ 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...f21c2ab. Read the comment docs.

sirewix · 2025-01-08T09:40:52Z

We should probably guard this behind cargo feature

src/telemetry.rs

src/telemetry/config.rs

src/telemetry/reqwest_middleware.rs

src/telemetry/config.rs

sirewix · 2025-01-09T15:55:39Z

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

mzaniolo · 2025-01-09T21:06:57Z

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

src/telemetry/config.rs

sirewix

LGTM aside from few cosmetic nits

sirewix · 2025-01-15T11:29:48Z

README.md

+
+For setup all that's needed it to run the function `famedly_rust_utils::famedly_rust_utils::telemetry::init_otel`. The function returns a guard that takes care of properly shutting down the providers.
+
+If no configuration is present the exporting of logs traces and metrics is disable and the stdout logging is enable.


Suggested change

If no configuration is present the exporting of logs traces and metrics is disable and the stdout logging is enable.

If no configuration is present the exporting of logs traces and metrics is disabled and the stdout logging is enabled.

sirewix · 2025-01-15T11:32:37Z

src/telemetry/config.rs

+}
+
+impl ProviderConfig {
+	#[allow(clippy::expect_used)]


Suggested change

#[allow(clippy::expect_used)]

sirewix · 2025-01-15T11:43:58Z

src/telemetry/mod.rs

+			self.logger_provider.as_ref().inspect(|logger_provider| {
+				if let Err(err) = logger_provider.shutdown() {
+					tracing::error!("Could not shutdown LoggerProvider: {err}");
+				}
+			});


IMHO: I think it makes more sense for if let syntax on self fields as it's more about flow control, while inspect and inspect_err semantially are more about "inspection", about some side effects that are not part of general flow, like tracing. Feel free to disagree, I would just write it like this:

Suggested change

self.logger_provider.as_ref().inspect(|logger_provider| {

if let Err(err) = logger_provider.shutdown() {

tracing::error!("Could not shutdown LoggerProvider: {err}");

}

});

if let Some(logger_provider) = self.logger_provider.as_ref() {

logger_provider.shutdown().inspect_err(|err|

tracing::error!("Could not shutdown LoggerProvider: {err}")

)

};

sirewix · 2025-01-15T11:48:03Z

README.md

+The functions on the crate exporting opentelemetry traces should be annotated with `tracing::instrument` to generate a new span for that function. Documentation on this macro can be found on the [here](https://docs.rs/tracing/latest/tracing/attr.instrument.html)
+
+The opentelemetry information is exported using gRPC to and opentelemetry collector. By default the expected endpoint is `http://localhots:4317`
+The default level of logging and traces is `info` and the default filter directive is `opentelemetry=off,tonic=off,h2=off,reqwest=info,axum=info,hyper=info,hyper-tls=info,tokio=info,tower=info,josekit=info,openssl=info`


This part is outdated, I suppose

sirewix · 2025-01-15T11:49:56Z

README.md

+
+### Propagate the context
+
+A context can be propagated to allow linking the traces from two different services. This is done by injecting the context information on the request and retrieving it on the other service.


Suggested change

A context can be propagated to allow linking the traces from two different services. This is done by injecting the context information on the request and retrieving it on the other service.

A context can be propagated to allow linking the traces from two different services. This is done by injecting the context information into the request and retrieving it in another service.

Still not sure if this is gramatically correct :/

sirewix · 2025-01-15T11:50:50Z

README.md

+
+#### reqwest
+
+For injecting the current context using the reqwest client we can warp a client on a [reqwest-middleware](https://crates.io/crates/reqwest-middleware) and use the `OtelMiddleware` middleware present on the crate.


Suggested change

For injecting the current context using the reqwest client we can warp a client on a [reqwest-middleware](https://crates.io/crates/reqwest-middleware) and use the `OtelMiddleware` middleware present on the crate.

For injecting the current context using the reqwest client we can wrap a client in a [reqwest-middleware](https://crates.io/crates/reqwest-middleware) and use the `OtelMiddleware` middleware present in this crate.

sirewix · 2025-01-15T11:51:56Z

README.md

+
+### Metrics
+
+For adding metrics all that it's needed it to make a trace with specific prefix. The documentation on how it works is [here](https://docs.rs/tracing-opentelemetry/latest/tracing_opentelemetry/struct.MetricsLayer.html#usage)


Suggested change

For adding metrics all that it's needed it to make a trace with specific prefix. The documentation on how it works is [here](https://docs.rs/tracing-opentelemetry/latest/tracing_opentelemetry/struct.MetricsLayer.html#usage)

For adding metrics all that is needed is to make a trace with specific prefix. The documentation on how it works is [here](https://docs.rs/tracing-opentelemetry/latest/tracing_opentelemetry/struct.MetricsLayer.html#usage)

sirewix · 2025-01-15T11:52:35Z

README.md

+
+For adding metrics all that it's needed it to make a trace with specific prefix. The documentation on how it works is [here](https://docs.rs/tracing-opentelemetry/latest/tracing_opentelemetry/struct.MetricsLayer.html#usage)
+
+For adding metrics to axum servers creates like [tower-otel-http-metrics](https://github.com/francoposa/tower-otel-http-metrics)


Suggested change

For adding metrics to axum servers creates like [tower-otel-http-metrics](https://github.com/francoposa/tower-otel-http-metrics)

For adding metrics to axum servers see crates like [tower-otel-http-metrics](https://github.com/francoposa/tower-otel-http-metrics)

mzaniolo requested a review from a team as a code owner January 6, 2025 11:39

mzaniolo requested a review from tlater-famedly January 6, 2025 11:39

mzaniolo force-pushed the mzaniolo/telemetry branch from 7eb6574 to 181e5cf Compare January 6, 2025 14:18

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

sirewix reviewed Jan 8, 2025

View reviewed changes

src/telemetry.rs Outdated Show resolved Hide resolved

mzaniolo force-pushed the mzaniolo/telemetry branch 2 times, most recently from 3bb5329 to 4c1d1df Compare January 8, 2025 13:04

sirewix reviewed Jan 8, 2025

View reviewed changes

src/telemetry/config.rs Outdated Show resolved Hide resolved

mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from 7e9a05f to ac11152 Compare January 8, 2025 17:48

mzaniolo requested a review from sirewix January 9, 2025 11:19

mzaniolo force-pushed the mzaniolo/telemetry branch 2 times, most recently from 06f3601 to 5ad809d Compare January 9, 2025 12:23

sirewix reviewed Jan 9, 2025

View reviewed changes

src/telemetry/reqwest_middleware.rs Outdated Show resolved Hide resolved

sirewix reviewed Jan 9, 2025

View reviewed changes

src/telemetry/config.rs Outdated Show resolved Hide resolved

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

mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from ea95e16 to 85d0e49 Compare January 10, 2025 13:15

mzaniolo changed the title ~~WIP: feat: Add OpenTelemetry setup function~~ feat: Add OpenTelemetry setup function Jan 10, 2025

mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from f8994cd to c7bf78c Compare January 10, 2025 13:52

mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from 9f8bb11 to 31bd81d Compare January 10, 2025 17:33

sirewix mentioned this pull request Jan 10, 2025

Add a "base" config #10

Open

mzaniolo force-pushed the mzaniolo/telemetry branch 2 times, most recently from c899ea2 to 4dce311 Compare January 13, 2025 11:30

mzaniolo linked an issue Jan 13, 2025 that may be closed by this pull request

Add a "base" config #10

Open

mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from ea2004a to cc1cd81 Compare January 13, 2025 12:22

sirewix reviewed Jan 13, 2025

View reviewed changes

src/telemetry/config.rs Outdated Show resolved Hide resolved

src/telemetry/config.rs Outdated Show resolved Hide resolved

mzaniolo force-pushed the mzaniolo/telemetry branch 3 times, most recently from 3ce66fe to 5f81383 Compare January 14, 2025 13:09

feat: Add OpenTelemetry setup function

f21c2ab

mzaniolo force-pushed the mzaniolo/telemetry branch from 5f81383 to f21c2ab Compare January 14, 2025 13:22

sirewix reviewed Jan 15, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add OpenTelemetry setup function #11

feat: Add OpenTelemetry setup function #11

mzaniolo commented Jan 6, 2025

codecov bot commented Jan 6, 2025 •

edited

Loading

sirewix commented Jan 8, 2025

sirewix commented Jan 9, 2025

mzaniolo commented Jan 9, 2025

sirewix left a comment

sirewix Jan 15, 2025

sirewix Jan 15, 2025

sirewix Jan 15, 2025

sirewix Jan 15, 2025

sirewix Jan 15, 2025

sirewix Jan 15, 2025

sirewix Jan 15, 2025

sirewix Jan 15, 2025


		For setup all that's needed it to run the function `famedly_rust_utils::famedly_rust_utils::telemetry::init_otel`. The function returns a guard that takes care of properly shutting down the providers.

		If no configuration is present the exporting of logs traces and metrics is disable and the stdout logging is enable.


		### Propagate the context

		A context can be propagated to allow linking the traces from two different services. This is done by injecting the context information on the request and retrieving it on the other service.


		#### reqwest

		For injecting the current context using the reqwest client we can warp a client on a [reqwest-middleware](https://crates.io/crates/reqwest-middleware) and use the `OtelMiddleware` middleware present on the crate.


		### Metrics

		For adding metrics all that it's needed it to make a trace with specific prefix. The documentation on how it works is [here](https://docs.rs/tracing-opentelemetry/latest/tracing_opentelemetry/struct.MetricsLayer.html#usage)

	For adding metrics all that it's needed it to make a trace with specific prefix. The documentation on how it works is [here](https://docs.rs/tracing-opentelemetry/latest/tracing_opentelemetry/struct.MetricsLayer.html#usage)
	For adding metrics all that is needed is to make a trace with specific prefix. The documentation on how it works is [here](https://docs.rs/tracing-opentelemetry/latest/tracing_opentelemetry/struct.MetricsLayer.html#usage)


		For adding metrics all that it's needed it to make a trace with specific prefix. The documentation on how it works is [here](https://docs.rs/tracing-opentelemetry/latest/tracing_opentelemetry/struct.MetricsLayer.html#usage)

		For adding metrics to axum servers creates like [tower-otel-http-metrics](https://github.com/francoposa/tower-otel-http-metrics)

	For adding metrics to axum servers creates like [tower-otel-http-metrics](https://github.com/francoposa/tower-otel-http-metrics)
	For adding metrics to axum servers see crates like [tower-otel-http-metrics](https://github.com/francoposa/tower-otel-http-metrics)

feat: Add OpenTelemetry setup function #11

Are you sure you want to change the base?

feat: Add OpenTelemetry setup function #11

Conversation

mzaniolo commented Jan 6, 2025

codecov bot commented Jan 6, 2025 • edited Loading

Codecov Report

sirewix commented Jan 8, 2025

sirewix commented Jan 9, 2025

mzaniolo commented Jan 9, 2025

sirewix left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

codecov bot commented Jan 6, 2025 •

edited

Loading