juliuskoval / NLog.Targets.OpenTelemetryProtocol

0 stars 3 forks source link

Adding support for Aspire environment variables ? #17

Open snakefoot opened 2 weeks ago

snakefoot commented 2 weeks ago

Maybe setup OpenTelemetry-Target to recognize these Environment-Variables by default unless nothing else as been assigned:

Where OTEL_EXPORTER_OTLP_ENDPOINT maps to Layout-property:

Where OTEL_EXPORTER_OTLP_PROTOCOL can have following values:

       if (options.Protocol == OtlpProtocol.HttpProtobuf && !string.IsNullOrEmpty(options.Endpoint) && !options.Endpoint.EndsWith("/v1/logs"))
            options.Endpoint = $"{options.Endpoint.TrimEnd('/')}/v1/logs";

Where OTEL_EXPORTER_OTLP_HEADERS + OTEL_RESOURCE_ATTRIBUTES must be parsed as comma-seperated mapping (key1=value1,key2=value2):

        foreach (var part in config?.Split(',') ?? [])
        {
                if (part.Split('=') is { Length: 2 } parts)
                    headers[parts[0.Trim()]] = parts[1].Trim();
        }
richbeales commented 1 week ago

This would also prevent the need to have our access token/key committed to source control (if we can pick up the OTLP headers from an env variable)

juliuskoval commented 1 week ago

I tested the latest version of the package and the environment variables were respected if I left the appropriate field empty in nlog.config, otherwise the value from the config was used. The only problem was that the environment variable for resources didn't work if I set UseDefaultResources to false, so I'll fix that.

@richbeales Could you test this?

juliuskoval commented 1 week ago

Actually, it looks like environment variables fall under default resources, so it looks like UseDefaultResources=false means they should be ignored. https://github.com/open-telemetry/opentelemetry-dotnet/blob/d8ce51b1b482e7432da0e7498fdc1052eb2ca807/src/OpenTelemetry/Resources/ResourceBuilder.cs#L57

snakefoot commented 5 days ago

Yes looks like it will automatically apply headers etc unless specified by NLog target:

Then I think my worries was wrong. Just need to update the documentation about environment-variables will be used by default, when not having specified anything on NLog-target (Using default value UseDefaultResources=true).

snakefoot commented 5 days ago

Created #18 to close this issue