microsoft / ApplicationInsights-node.js

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

[3.0.0-beta.11] Incorrect types for `useAzureMonitor`? #1269

Closed mderriey closed 2 months ago

mderriey commented 5 months ago

Hi 👋

We've updated to 3.0.0-beta.11 to take advantage of errors being correctly serialised.

However, we're facing an issue with types. If we understand correctly, this library extends the InstrumentionOptions type from `@azure/opentelemetry-monitor so we can specify instrumentations from both the OTel distro and this library in a single object.

However, this type isn't used, so this code doesn't compile with TypeScript:

const resource = Resource.EMPTY
resource.attributes[SemanticResourceAttributes.SERVICE_NAME] = 'service-name'
resource.attributes[SemanticResourceAttributes.SERVICE_NAMESPACE] = 'service-namespace'

useAzureMonitor({
  resource,
  instrumentationOptions: {
    azureSdk: { enabled: false },
    http: { enabled: false },
    mongoDb: { enabled: false },
    mySql: { enabled: false },
    postgreSql: { enabled: false },
    redis: { enabled: false },
    redis4: { enabled: false },
    winston: { enabled: true },
    console: { enabled: false },
    bunyan: { enabled: false },
  }
})

The TypeScript error is:

TS2353: Object literal may only specify known properties, and winston does not exist in type InstrumentationOptions

We could only fix it by juggling types as below:

import type { InstrumentationOptions as ApplicationInsightsInstrumentationOptions } from 'applicationinsights/out/src/types'
import type { InstrumentationOptions as OpenTelemetryInstrumentationOptions } from '@azure/monitor-opentelemetry'

// Default attributes set on the OpenTelemetry traces
const resource = Resource.EMPTY
resource.attributes[SemanticResourceAttributes.SERVICE_NAME] = 'service-name'
resource.attributes[SemanticResourceAttributes.SERVICE_NAMESPACE] = 'service-namespace'

useAzureMonitor({
  resource,
  instrumentationOptions: {
    azureSdk: { enabled: false },
    http: { enabled: false },
    mongoDb: { enabled: false },
    mySql: { enabled: false },
    postgreSql: { enabled: false },
    redis: { enabled: false },
    redis4: { enabled: false },
    winston: { enabled: true },
    console: { enabled: false },
    bunyan: { enabled: false },
  } satisfies ApplicationInsightsInstrumentationOptions as unknown as OpenTelemetryInstrumentationOptions,
})

Are we doing something wrong, or is there another recommended way to set this up?

JacksonWeber commented 4 months ago

@mderriey Above PR should resolve this issue for you and be available in the next release. Thank you for reporting the issue!

mderriey commented 2 months ago

We updated to 3.0.0-beta.12 and the issue is still present.

λ npm ls applicationinsights
<redacted>@1.0.0 <redacted>
`-- applicationinsights@3.0.0-beta.12

Screenshot of TypeScript error:

image

useAzureMonitor is the one from applicationinsights, not @azure/monitor-opentelemetry.

Code snippet if you want to try yourself:

import { Resource } from '@opentelemetry/resources'
import { SEMRESATTRS_SERVICE_NAME, SEMRESATTRS_SERVICE_NAMESPACE } from '@opentelemetry/semantic-conventions'
import { useAzureMonitor } from 'applicationinsights'

// Default attributes set on the OpenTelemetry traces
const resource = Resource.EMPTY
resource.attributes[SEMRESATTRS_SERVICE_NAME] = '<redacted>'
resource.attributes[SEMRESATTRS_SERVICE_NAMESPACE] = '<redacted>'

useAzureMonitor({
  resource,
  instrumentationOptions: {
    azureSdk: { enabled: false },
    http: { enabled: false },
    mongoDb: { enabled: false },
    mySql: { enabled: false },
    postgreSql: { enabled: false },
    redis: { enabled: false },
    redis4: { enabled: false },
    winston: { enabled: true },
    console: { enabled: false },
    bunyan: { enabled: false },
  },
})
JacksonWeber commented 2 months ago

Closing as the unification of the beta into Application Insights 3.0.0 means that this project is intended to be used with the legacy application insights API.