Add ability to control metric output level through spin_telemetry cargo feature

[rylev]

This gives embedders a way to control at which level telemetry should be emitted at. I think this is the best way to allow metrics which are emitted deep from within Spin's codebase to still be controlled by embedders which may choose to statically strip out traces and would thus never see these metrics.

[lann]

It would be a larger change but I'd be inclined to switch metrics to use opentelemetry directly rather than through the wonky tracing interface.

[lann]

FWIW we do already have cfg-if in the dep tree...

Looking forward to bumping the MSRV too.

[lann]

Given that these are sort of "logically additive" it would be nice to treat this the same as just enabling metrics-default-level-info so that --all-features doesn't break.

[lann]

...though annoyingly that already appears to be broken by llm-metal / llm-cublas.

[calebschoepp]

It would be a larger change but I'd be inclined to switch metrics to use opentelemetry directly rather than through the wonky tracing interface.

I share the same desire, but not blocking of course.

[rylev]

It would be a larger change but I'd be inclined to switch metrics to use opentelemetry directly rather than through the wonky tracing interface.

@lann @calebschoepp I'd be fine with this, but that would be a breaking change. Not sure how we want to handle that.

[calebschoepp]

@lann @calebschoepp I'd be fine with this, but that would be a breaking change. Not sure how we want to handle that.

Breaking change for embedders, or for something else I'm misunderstanding?

[lann]

Not if they're using the macros! 😅

[rylev]

For all users of spin. It's not a breaking code change, but if we use opentelemetry directly, users of spin will suddenly start seeing metrics even if they had previously filtered them out with RUST_LOG=info before.

[lann]

I think you already have to opt-in with env vars don't you?

spin/crates/telemetry/src/env.rs

Lines 29 to 41 in ae349c2

	/// Returns a boolean indicating if the OTEL metrics layer should be enabled.
	///
	/// It is considered enabled if any of the following environment variables are set and not empty:
	/// - `OTEL_EXPORTER_OTLP_ENDPOINT`
	/// - `OTEL_EXPORTER_OTLP_METRICS_ENDPOINT`
	///
	/// Note that this is overridden if OTEL_SDK_DISABLED is set and not empty.
	pub fn otel_metrics_enabled() -> bool {
	any_vars_set(&[
	OTEL_EXPORTER_OTLP_ENDPOINT,
	OTEL_EXPORTER_OTLP_METRICS_ENDPOINT,
	]) && !otel_sdk_disabled()
	}

[calebschoepp]

For all users of spin. It's not a breaking code change, but if we use opentelemetry directly, users of spin will suddenly start seeing metrics even if they had previously filtered them out with RUST_LOG=info before.

I personally don't see this as a problem. I'd be really surprised if people were explicitly using RUST_LOG to filter out metrics while still opting into forwarding stuff to the OTel collector. The fact that metrics can be filtered out by RUST_LOG is just a weird side effect of our implementation that I think we should move away from.

[itowlson]

I see two approvals but a lot of ongoing discussion. Should I merge this or are folks thinking there is more work/thought needed?

[rylev]

@itowlson I've provided an alternative implementation in #3623 - let's see what @lann and @calebschoepp think

[rylev]

Closing in favor of #3623