microsoft / ApplicationInsights-node.js

Microsoft Application Insights SDK for Node.js
MIT License
320 stars 138 forks source link

[3.0.0-beta.7] Cannot add instrumentations anymore #1182

Closed mderriey closed 10 months ago

mderriey commented 11 months ago

Hi there,

With 3.0.0-beta.7 consuming @azure/monitor-opentelemetry, we can't add additional instrumentations, which is a step back feature-wise.

Nothing is exposed on the TelemetryClient class, and the AzureMonitorOpenTelemetryClient class doesn't expose its TraceHandler instance.

I suspect this should be an issue on the azure-sdk-for-js repo, but I thought I'd open it here in case you have plans to expose it somehow through TelemetryClient.

Cheers.

cuzzlor commented 11 months ago

I came here wondering if the [Object object] output has been fixed, I guess beta.7 is not the one 😭

In my project, we are currently using:

const client = new ApplicationInsightsClient(config)

client.getTraceHandler().addInstrumentation(new ExpressInstrumentation())
client.getTraceHandler().addInstrumentation(new GraphQLInstrumentation())
client.getTraceHandler().addInstrumentation(new TediousInstrumentation())
client.getTraceHandler().addInstrumentation(new IORedisInstrumentation())
gidich commented 10 months ago

@mderriey - Thank you for posting this issue - I'm hoping that functionality will return in a future beta, for now, we'll be staying at beta.6.

heruwala commented 10 months ago

I had to downgrade to beta.6. Please allow adding instrumentations.

hectorhdzg commented 10 months ago

Instrumentation can still be added in beta.7, the way to do it is slightly different, we are going though some refactoring and API reviews that will change how this SDK is consumed, but features should continue to exist.

const { ApplicationInsightsClient } = require("applicationinsights");
const { trace } = require('@opentelemetry/api');
const { registerInstrumentations } = require("@opentelemetry/instrumentation");
const { WinstonInstrumentation } = require("@opentelemetry/instrumentation-winston");

const client = new ApplicationInsightsClient({})
// Get global tracerProvider registered during ApplicationInsightsClient initialization
const tracerProvider = trace.getTracerProvider().getDelegate(); 
registerInstrumentations({
    instrumentations: new WinstonInstrumentation(),
    tracerProvider: tracerProvider
});
mderriey commented 10 months ago

Thanks @hectorhdzg, apologies for replying a bit late, I've moved on to another project.

Your solution worked nicely, thanks 🎉 It's nice to see App Insights / Azure Monitor SDKs come closer to vanilla OTel.

In case it helps others, when using TypeScript, I had to slap a couple type casts to get the tracer provider, i.e.:

const tracerProvider = (trace.getTracerProvider() as ProxyTracerProvider).getDelegate() as NodeTracerProvider

That was also useful to add span processors, as addSpanProcessor is not available on the base TracerProvider interface.