Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Use new UseOtlpExporter method from the OTEL library #3017

Closed
davidfowl opened this issue Mar 19, 2024 · 12 comments
Closed

Use new UseOtlpExporter method from the OTEL library #3017

davidfowl opened this issue Mar 19, 2024 · 12 comments

Comments

@davidfowl
Copy link
Member

open-telemetry/opentelemetry-dotnet#5400

@DamianEdwards
Copy link
Member

DamianEdwards commented Mar 19, 2024

Currently available in 1.8.0-beta.1

This would be an update to the templates IIUC, specifically the ServiceDefaults project. We should wait until there's a stable release before integrating I think.

@CodeBlanch
Copy link

Really the whole point of putting out that beta was for Aspire to take it for a spin and provide feedback 🤣 Please do!

Here's the things in 1.8.0-beta.1 which should be of interest to Aspire:

  • Addition of UseOtlpExporter
  • Removal of OTEL_DOTNET_EXPERIMENTAL_OTLP_EMIT_EXCEPTION_LOG_ATTRIBUTES
  • Addition of OTEL_DOTNET_EXPERIMENTAL_OTLP_ENABLE_INMEMORY_RETRY

This one didn't make the first beta but will come in the next drop: open-telemetry/opentelemetry-dotnet#5448

@DamianEdwards
Copy link
Member

@CodeBlanch great! I'll try it out to get a sense of how it would impact our experience, thanks.

@DamianEdwards
Copy link
Member

OK so yeah, this allows us to simplify the ServiceDefaults code to enable OTLP quite a bit:

private static IHostApplicationBuilder AddOpenTelemetryExporters(this IHostApplicationBuilder builder)
{
+    builder.Services.AddOpenTelemetry()
+       // Enable OTLP exporter for all telemetry based on defaults and environment variables
+       .UseOtlpExporter();
+       // Uncomment the following lines to enable the Prometheus exporter (requires the OpenTelemetry.Exporter.Prometheus.AspNetCore package)
+       //.WithMetrics(metrics => metrics.AddPrometheusExporter());
    
-    var useOtlpExporter = !string.IsNullOrWhiteSpace(builder.Configuration["OTEL_EXPORTER_OTLP_ENDPOINT"]);
    
-    if (useOtlpExporter)
-    {
-        builder.Services.Configure<OpenTelemetryLoggerOptions>(logging => logging.AddOtlpExporter());
-        builder.Services.ConfigureOpenTelemetryMeterProvider(metrics => metrics.AddOtlpExporter());
-        builder.Services.ConfigureOpenTelemetryTracerProvider(tracing => tracing.AddOtlpExporter());
-    }

-    // Uncomment the following lines to enable the Prometheus exporter (requires the OpenTelemetry.Exporter.Prometheus.AspNetCore package)
-    // builder.Services.AddOpenTelemetry()
-    //    .WithMetrics(metrics => metrics.AddPrometheusExporter());
    
    // Uncomment the following lines to enable the Azure Monitor exporter (requires the Azure.Monitor.OpenTelemetry.AspNetCore package)
    //if (!string.IsNullOrEmpty(builder.Configuration["APPLICATIONINSIGHTS_CONNECTION_STRING"]))
    //{
    //    builder.Services.AddOpenTelemetry()
    //       .UseAzureMonitor();
    //}

     return builder;
}

@DamianEdwards
Copy link
Member

I'll move this to preview.6 as I'm not expecting we're going to get a stable version of the package with this change in time for preview.5

@DamianEdwards
Copy link
Member

@CodeBlanch
Copy link

@DamianEdwards Does Aspire use something like appSettings.Development.json? Asking because FYI using rc.1 you could remove this...

if (builder.Environment.IsDevelopment())
{
// We want to view all traces in development
tracing.SetSampler(new AlwaysOnSampler());
}

...and do it via config:

appSettings.Development.json

{
   "OTEL_TRACES_SAMPLER": "always_on"
}

If you wanted to 😄

@davidfowl
Copy link
Member Author

We can set environment variables.

cc @JamesNK

@samsp-msft
Copy link
Member

#3316

@CodeBlanch
Copy link

CodeBlanch commented Apr 3, 2024

Stable 1.8.0 OTel packages should be on Nuget now.

One late change to pay attention to: https://github.com/open-telemetry/opentelemetry-dotnet/blob/fb74013d644d56f44ef89dd57358947bd9f68345/src/OpenTelemetry.Exporter.OpenTelemetryProtocol/CHANGELOG.md?plain=1#L17-L22

The envar key/value for enabling in-memory retry changed for stable.

@davidfowl davidfowl assigned JamesNK and unassigned DamianEdwards Apr 6, 2024
@davidfowl
Copy link
Member Author

Done in #3359

@martincostello
Copy link
Member

FYI I've found a bunch of trim warnings related to this: open-telemetry/opentelemetry-dotnet#5518 (comment)

@github-actions github-actions bot locked and limited conversation to collaborators May 9, 2024
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

6 participants