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

[Beta] Error that @azure/core-tracing is loaded before @azure/opentelemetry-instrumentation-azure-sdk #1107

Open
marcus13371337 opened this issue Mar 21, 2023 · 10 comments

Comments

@marcus13371337
Copy link

Hello,

I'm using the latest beta-version of this package, however, on startup, I'm getting the following error:

ApplicationInsights:@azure/opentelemetry-instrumentation-azure-sdk [
  'Module @azure/core-tracing has been loaded before @azure/opentelemetry-instrumentation-azure-sdk so it might not work, please initialize it before requiring @azure/core-tracing'
]

I don't have those packages as a dependency, and I don't import/require them anywhere, so I guess the error originates from this package. Is this a bug?

@MikaelEdebro
Copy link

I got the same warning when trying out the beta package.

@dariusmb
Copy link

Hello. I get the same error.

Are there any updates on this?

@hectorhdzg
Copy link
Member

This is not an error just a warning, Azure SDK auto instrumentation should be functional even if the warning is triggered, please ignore for now, this warning would be gone once we add a LogExporter and remove internal @Azure package dependencies in this project

@ericis
Copy link

ericis commented Jan 3, 2024

This is not an error just a warning, Azure SDK auto instrumentation should be functional even if the warning is triggered, please ignore for now, this warning would be gone once we add a LogExporter and remove internal @Azure package dependencies in this project

@hectorhdzg this actually appears in my program's output. We are generating a Node CLI that uses command telemetry through the SDK. So, this so-called "warning" is not simply output to logs on a server, but actually written out to the console whenever any command is executed.

How can we avoid this?

@hectorhdzg
Copy link
Member

@ericis this is an internal log, so you should be able to control it changing the log level, you can also change the log destination to be file and console would not be written.

https://github.com/microsoft/ApplicationInsights-node.js/tree/beta?tab=readme-ov-file#self-diagnostics

@ericis
Copy link

ericis commented Jan 4, 2024

Thank you @hectorhdzg . It seems it is not possible to override the logging in the usual way.

I was surprised that the "@azure/monitor-opentelemetry" package has it's own InternalAzureLogger and doesn't seem to make use of the existing "@azure/logger" instance.

As a result, there are THREE logging overrides to be successful while using the "@azure/opentelemetry-instrumentation-azure-sdk" library.

Override "@opentelemetry/api" logging

import api, {
  DiagLogger,
  DiagLoggerOptions,
} from '@opentelemetry/api'

const openTelemetryLogger: DiagLogger = {
  debug: (message: string, ...args: unknown[]) => {
    // no-op example
  },
  error: (message: string, ...args: unknown[]) => {
    // no-op example
  },
  info: (message: string, ...args: unknown[]) => {
    // no-op example
  },
  verbose: (message: string, ...args: unknown[]) => {
    // no-op example
  },
  warn: (message: string, ...args: unknown[]) => {
    // no-op example
  },
}

const openTelemetryLoggerOptions: DiagLoggerOptions = {
  suppressOverrideMessage: true,
}

api.diag.setLogger(openTelemetryLogger, openTelemetryLoggerOptions)

Override "@azure/logger" logging

import { AzureLogger } from '@azure/logger'

AzureLogger.log = (...args: any[]) => {
  // no-op example
}

Override "@azure/opentelemetry-instrumentation-azure-sdk" logging

This is not currently possible. Instead, all warnings have to be ignored in order to ignore this one message or logging has to be redirected to an output file in order to suppress it from the console.

This is inconsistent with the OpenTelemetry logging and Azure SDK logging solutions and does not provide the developer with enough control.

process.env.APPLICATIONINSIGHTS_INSTRUMENTATION_LOGGING_LEVEL = "INFO";
process.env.APPLICATIONINSIGHTS_LOG_DESTINATION = "file";
process.env.APPLICATIONINSIGHTS_LOGDIR = "C:/applicationinsights/logs";

@ericis
Copy link

ericis commented Jan 4, 2024

Given the current limitations of the "@azure/opentelemetry-instrumentation-azure-sdk" logging, I was able to workaround this issue with a fairly significant flaw that logging details will be treated very differently for this specific library than anything else in the application. This also limits our future ability to redirect these log entries and mandates that the developers are all aware that there is a separate "appinsights" log file for the application.

// map to AppInsights-specific log levels
const getAppInsightsLogLevel = (logLevel: LogLevel) => {
  // NONE, ERROR, WARN, INFO, DEBUG, VERBOSE and ALL
  // https://github.com/microsoft/ApplicationInsights-node.js/tree/beta?tab=readme-ov-file#self-diagnostics
  switch (logLevel) {
    case LogLevel.debug:
      return 'DEBUG'
    case LogLevel.error:
      return 'ERROR'
    case LogLevel.info:
      return 'INFO'
    case LogLevel.warn:
      return 'WARN'
    default:
      return 'NONE'
  }
}

// BUG: AppInsights and Azure OpenTelemetry do not offer the ability to
//      override logging. As a result, the only option to prevent log
//      details from "spilling" into the console is to send it to a file.
//      Of course, this also results in important information like Errors
//      being sent to a separate log file with no ability to control how
//      they are reported.
const appInsightsLogFilePath = path.join(
  options.config.dataDir,
  'my-cli.appinsights.log'
)


process.env.APPLICATIONINSIGHTS_INSTRUMENTATION_LOGGING_LEVEL =
  getAppInsightsLogLevel(activeLogLevel)
process.env.APPLICATIONINSIGHTS_LOG_DESTINATION = 'file'
process.env.APPLICATIONINSIGHTS_LOGDIR = appInsightsLogFilePath

@hectorhdzg
Copy link
Member

@ericis I'm a little bit confused about what is your specific need, we do in fact have multiple loggers involved, as we use both OpenTelemetry and Azure SDK packages in our SDK, I can see level works as expected when using application insights config, we do not support filtering out specific logs so there is no way to filter out the current warning without changing the level to error. Can you describe your scenario?, are you using other Azure SDKs and expecting to be able to override Azure SDK logger?, we are working on improving internal logging here, I can see we have some flaws but understanding your issue better could help us to include it as well.

@ericis
Copy link

ericis commented Jan 5, 2024

@ericis I'm a little bit confused about what is your specific need, we do in fact have multiple loggers involved, as we use both OpenTelemetry and Azure SDK packages in our SDK, I can see level works as expected when using application insights config, we do not support filtering out specific logs so there is no way to filter out the current warning without changing the level to error. Can you describe your scenario?, are you using other Azure SDKs and expecting to be able to override Azure SDK logger?, we are working on improving internal logging here, I can see we have some flaws but understanding your issue better could help us to include it as well.

Thanks @hectorhdzg . Sorry for the confusion.

If I were to take a step back, while I can't speak for all developers, I might hypothesize that developer joy when using OpenTelemetry + Azure would be to assume that overriding the DiagLogger provided by OpenTelemetry would be sufficient to provide logging for the entire application and that Microsoft would internally wire-up logging like AzureLogger and Application Insights internal logging to behave accordingly. In other words, if a developer were to implement OpenTelemetry to send ALL logging to a single ReST web service endpoint, then all Microsoft Azure implementation code would assume to do the same thing.

However, I have some empathy toward the fact that AzureLogger is super generic (too generic) for overriding all Azure-related logging activities (except AppInsights in this case, which has its own internal logging solution and does not seem to even know about AzureLogger... which is odd to me by itself). I also have empathy toward the fact that AppInsights logging has to work independently of OpenTelemetry as well. For both of these, my personal suggestion would be to align the extensibility of these logging solutions to allow for developer control and overrides as well as to work toward a unified development interface for logging in much the same way that OpenTelemetry itself is still working toward a global standard. As a bonus, it would be great if Microsoft simply put it's weight behind the standard without any customization other than backwards-compatible "shims" for existing libraries.

@hectorhdzg
Copy link
Member

Thanks @ericis for extra details, I created following PR related to that
Azure/azure-sdk-for-js#28183

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

No branches or pull requests

5 participants