apm-agent-dotnet icon indicating copy to clipboard operation
apm-agent-dotnet copied to clipboard

Enable global file logging

Open Mpdreamz opened this issue 1 year ago • 3 comments

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:

  • ELASTIC_OTEL_FILE_LOG_LEVEL (to anything but none)
  • ELASTIC_OTEL_FILE_LOG_DIRECTORY
  • ELASTIC_OTEL_LOG_TARGETS (to anything but none)
    • if only set to stdout no file will be created but global logging will kicking )

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

For backwards compatible reasons the profiler variables are also supported:

  • ELASTIC_APM_PROFILER_LOG
  • ELASTIC_APM_PROFILER_LOG_DIR
  • ELASTIC_APM_PROFILER_LOG_TARGETS

Globally setting

  • ELASTIC_APM_LOG_LEVEL and ELASTIC_APM_LOG_DIRECTORY is also supported but not preferred.

Setting these in any of our supported deploy scenarios:

  • Manual instrumentation (nuget)
    • ASP.NET (classic)
    • ASP.NET core
      • NOTE: we now log to both the configured ILogger and our global logging.
  • Auto Instrumentation
    • Profiler
    • Startup hooks To keep support existing deploys this now always globally logs to file if only ELASTIC_APM_STARTUP_HOOKS_LOGGING is set as well.

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 avatar Jun 03 '24 15:06 Mpdreamz

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:

  • ELASTIC_APM_LOG_LEVEL
  • ELASTIC_APM_LOG_DIRECTORY
  • ELASTIC_APM_LOG_TARGETS

For both? Or:

  • ELASTIC_APM_FILE_LOG_LEVEL
  • ELASTIC_APM_FILE_LOG_DIRECTORY
  • ELASTIC_APM_LOG_TARGETS

Mpdreamz avatar Jun 04 '24 08:06 Mpdreamz

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?

  • ELASTIC_OTEL_LOG_LEVEL
  • ELASTIC_OTEL_FILE_LOG_DIRECTORY
  • ELASTIC_OTEL_LOG_TARGETS

stevejgordon avatar Jun 04 '24 08:06 stevejgordon

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:

  • ELASTIC_OTEL_LOG_LEVEL
  • ELASTIC_OTEL_LOG_DIRECTORY
  • ELASTIC_OTEL_LOG_TARGETS

I like symmetry in the naming and its more terser 😸

Mpdreamz avatar Jun 04 '24 09:06 Mpdreamz