elastic / apm-agent-dotnet

https://www.elastic.co/guide/en/apm/agent/dotnet/current/index.html
Apache License 2.0
586 stars 208 forks source link

Enable global file logging #2371

Closed Mpdreamz closed 4 months ago

Mpdreamz commented 6 months ago

This ensures we can specify file logging globally and uniformly. Greatly simplifying the debug-ability of the agent.

The preferred way is to set any or all of the following:

This ensure we have one way to debug both our proprietary agent as well as the Elastic OpenTelemetry Distribution.

See: https://github.com/elastic/elastic-otel-dotnet/pull/129

For backwards compatible reasons the profiler variables are also supported:

Globally setting

Setting these in any of our supported deploy scenarios:

This further updates our docs for the profiler and troubleshooting to prefer ELASTIC_OTEL_* variables.

The profiler is updated to read the same environment variables as managed code.

Mpdreamz commented 6 months ago

A couple of minor fix suggestions. The only thing I'm a little unsure about is using the ELASTIC_OTEL_ env vars for this agent. I like that migration is easier, but not sure if it's subtly confusing why OTEL is mentioned in their names.

Aye, it's not ideal but I'd love to be in a situation where we can point customers to 3 variables that work across the board and in mixed environments.

We could use:

For both? Or:

stevejgordon commented 6 months ago

Yeah, okay. If we want one set to work for all things, let's go with the OTEL-based names, as that's where we expect to end up eventually. Would these make sense, since log level can apply to other loggers that may not be file-based?

Mpdreamz commented 6 months ago

Yeah it might make sense ELASTIC_OTEL_FILE_LOG_LEVEL on its own is enough to start logging to file today.

ELASTIC_OTEL_LOG_LEVEL on its own might be slightly confusing because the default for ELASTIC_OTEL_LOG_TARGETS is to include file.

However I think its benefits negate the possible confusion.

If we take this further we can also rename ELASTIC_OTEL_FILE_LOG_DIRECTORY to ELASTIC_OTEL_LOG_DIRECTORY since this only makes sense to file logging anyhow.

I'll update the PR to the following set:

I like symmetry in the naming and its more terser 😸