aws-observability / aws-otel-dotnet-instrumentation

Apache License 2.0
0 stars 6 forks source link

Fix the OTEL_TRACES_SAMPLER_ARG before executing sampler #103

Open birojnayak opened 2 months ago

birojnayak commented 2 months ago

Based on this specification here , we should define OtelTracesSamplerArg (OTEL_TRACES_SAMPLER_ARG) into three parts (or at least assume people might provide). First things you should do is, split into 3 parts, if any part has error, set the default value, afterwards execute below.

        string? tracesSampler = System.Environment.GetEnvironmentVariable(OtelTracesSampler);
        string? tracesSamplerArg = System.Environment.GetEnvironmentVariable(OtelTracesSamplerArg);
        double samplerProbability = 1.0;
        if (tracesSampler != null)
        {
            try
            {
                samplerProbability = Convert.ToDouble(tracesSamplerArg);
            }
            catch (Exception)
            {
                Logger.Log(LogLevel.Trace, "Could not convert OTEL_TRACES_SAMPLER_ARG to double. Using default value 1.0.");
            }
        }
AsakerMohd commented 2 months ago

I don't think there's anything to be done here. The 3 parts in the spec is mentioned for For jaeger_remote and parentbased_jaeger_remote and not xray (which we have our own guidance for about using endpoint and polling_interval). The jaeger ones use pollingIntervalMs instead of polling_interval for example. If you set them incorrectly (like using the jaeger ones instead of the xray ones), then it's expected that the distro would throw an error in this case.

birojnayak commented 2 months ago

I got confused as the behavior for xray sampler is bit different to other sampler. Please document in the repo or some external documentation on how to use, so that customers are not confused (just like me). Ideally I would prefer to be documented in readme file.