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

Consider implementing OpenTelemetry for Telmetry/Metrics #12251

Open
ShaneCourtrille opened this issue Aug 23, 2022 · 8 comments
Open

Consider implementing OpenTelemetry for Telmetry/Metrics #12251

ShaneCourtrille opened this issue Aug 23, 2022 · 8 comments

Comments

@ShaneCourtrille
Copy link
Contributor

Is your feature request related to a problem? Please describe.

There are a lot of processes within Orchard Core which are a complete black box once they are deployed. While we do get some logging, we are missing telemetry and metrics which are two critical things for supporting the system and understanding what is going on.

Describe the solution you'd like

Implement telemetry and metrics using OpenTelemetry since it now appears to have become an acceptable standard that isn't tied to a specific technology like Application Insights but can still be used with it and other tools that are available.

I think it makes the most sense for someone from the core team to implement the basic implementation with the addition of telemetry/metrics to one area of code so that over time the community could then provide additional PRs which add to other areas of the code.

@sebastienros sebastienros added this to the 1.x milestone Aug 25, 2022
@hishamco
Copy link
Member

FYI Lombiq already have Application Insight module https://github.com/Lombiq/Orchard-Azure-Application-Insights/tree/dc5403cca0d4965100cd75981cc5f74d33d6eb94 but it's doable to use OpenTelemetry in Orachard Core as core module or a community project

@MichaelPetrinolis
Copy link
Contributor

MichaelPetrinolis commented Aug 31, 2022

I like the idea of OpenTelemetry. Net Core provides integration for logging,tracing and metrics through the well-known APIs (System.Diagnostics and Logging). So, we just need to add some OC specific metrics and traces and an option to enable them.

Then in the app, we could use the OTEL libraries to enable trace, metrics and or logging and add exporters.
ex. Enable aspnetcore and httpclient instrumentation, the prometheus exporter and you are ready, you have traces and metrics.
Add also Grafana and voila, dashboards.

ex.
OrchardCore.AddOpenTelemetry() and in the OpenTelemetry configuration define the OrchardCore.Metrics Meter and configure the trace.

We must define what kind of metrics/traces specific for OC we should add.

@hishamco
Copy link
Member

I remembered Lombiq added Application Insights, so let me ping @Piedone if he can give us a brief though about what they did, then we can agree on something useful for OC

@Piedone
Copy link
Member

Piedone commented Aug 31, 2022

In my understanding, the Azure Application Insights (AI) service (i.e. the one you can use from the Azure Portal) already supports ingesting OpenTelemetry data, though its SDK instruments the app in its proprietary way, so it's two parallel systems. We could add OpenTelemetry instrumentation for some of the data our AI module collects, see here. I don't see these clashing though, since you either use OpenTelemetry or AI instrumentation.

@MichaelPetrinolis
Copy link
Contributor

I think the suggestion here is to add specific OC tracing and metrics in the core through the NetCore APIs (System.Diagnostics, we already support logging) and leave the selection to the user to select ApplicationInsights, Prometheus or OpenTelemetry and its exporters to connect to the monitoring tool of their choice. We should also provide documentation and examples on how to enable tracing and metrics through OpenTelemetry libs.

@sebastienros
Copy link
Member

@MichaelPetrinolis so you are suggesting we only need to provide some pointers to document how to export logging to OpenTelemetry? And just add more structured log where necessary?

@MichaelPetrinolis
Copy link
Contributor

@sebastienros exactly, enrich code with custom metrics and traces on critical services, and provide a nice tutorial on how to enable OpenTelemetry trace and metrics and how to export data to prometheus/grafana or whatever exporter the user prefers.
eg. code

        return Host.CreateDefaultBuilder(args)
            .ConfigureLogging(logging => logging.ClearProviders())
            .UseSerilog((hostingContext, configBuilder) =>
            {
                configBuilder.ReadFrom.Configuration(hostingContext.Configuration)
                .Enrich.WithEnvironmentName()
                .Enrich.WithMachineName()
                .Enrich.FromLogContext();
            })
            .ConfigureServices((app, services) =>
            {
                var prometheusPath = app.Configuration.GetSection("OpenTelemetry:Prometheus:ScrapeEndpointPath").Get<string>();
                var prometheusUriPrefixes = app.Configuration.GetSection("OpenTelemetry:Prometheus:UriPrefixes").Get<string>();

                var meterProviderBuilder = Sdk.CreateMeterProviderBuilder()
                    .ConfigureResource(x => x.AddService(Program.EndpointName, typeof(Program).Namespace, Program.EndpointVersion))
                    .AddMeter("NServiceBus.Core")
                    .AddAspNetCoreInstrumentation()
                    .AddHttpClientInstrumentation()
                    .AddPrometheusHttpListener(opt =>
                    {
                        if (!string.IsNullOrWhiteSpace(prometheusUriPrefixes))
                        {
                            opt.UriPrefixes = prometheusUriPrefixes.Split(new char[] { ';' }, StringSplitOptions.RemoveEmptyEntries);
                        }
                        if (!string.IsNullOrWhiteSpace(prometheusPath))
                        {
                            opt.ScrapeEndpointPath = prometheusPath;
                        }
                        Log.Information("Prometheus exporter is bound on {0} at path {1}", string.Join(';', opt.UriPrefixes), opt.ScrapeEndpointPath);
                    });

                services.AddSingleton(meterProviderBuilder.Build());
            })
           .ConfigureWebHostDefaults(webBuilder => webBuilder
                .UseStartup<Startup>())
            .Build();

@ShaneCourtrille
Copy link
Contributor Author

One thing to consider as well is the use of Span's for tracking the life cycle of background jobs or other activities that may be of interest.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

5 participants