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] - Nested objects get serialised as "[object Object]" again #1304

Open
cuzzlor opened this issue Apr 10, 2024 · 11 comments
Open

[Beta] - Nested objects get serialised as "[object Object]" again #1304

cuzzlor opened this issue Apr 10, 2024 · 11 comments

Comments

@cuzzlor
Copy link

cuzzlor commented Apr 10, 2024

Hi,

We are using winston logging + application insights beta.

v beta.12 is failing to serialise nested objects
image

v beta.11 still works
image

Similar to the last time in behaviour: #1169

Dependencies which work:

        "@opentelemetry/api": "^1.8.0",
        "@opentelemetry/instrumentation": "^0.50.0",
        "@opentelemetry/instrumentation-express": "^0.37.0",
        "@opentelemetry/instrumentation-graphql": "^0.39.0",
        "@opentelemetry/instrumentation-ioredis": "^0.39.0",
        "@opentelemetry/instrumentation-tedious": "^0.9.0",
        "@opentelemetry/instrumentation-winston": "^0.36.0",
        "@opentelemetry/sdk-trace-node": "^1.23.0",
        "@opentelemetry/semantic-conventions": "^1.23.0",
        "@opentelemetry/winston-transport": "^0.2.0",
        "applicationinsights": "3.0.0-beta.11",

Using the same deps with beta.12 results in nested objects being serialised as [object Object]

Full config used:

const resource = Resource.EMPTY
resource.attributes[SEMRESATTRS_SERVICE_NAME] = '[REDACTED]'
resource.attributes[SEMRESATTRS_SERVICE_NAMESPACE] = '[REDACTED]'

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

    http: { enabled: true },
    winston: { enabled: true },
  } satisfies ApplicationInsightsInstrumentationOptions as unknown as OpenTelemetryInstrumentationOptions, // https://github.com/microsoft/ApplicationInsights-node.js/issues/1269
})

const tracerProvider = trace.getTracerProvider()

registerInstrumentations({
  tracerProvider,
  instrumentations,
})
@JacksonWeber
Copy link
Contributor

@cuzzlor Thank you for providing the config and reporting this issue. Could you also provide an example of a log producing the output in the portal you noted above? Thank you!

@cuzzlor
Copy link
Author

cuzzlor commented Apr 15, 2024

@JacksonWeber An example of producing a log which would result in a field of the trace customDimensions section showing a value of [object Object] would be any that uses an object as a value in the winston logger 2nd log method parameter for log entry arguments, e.g.:

logger.info('This is my info log message', { foo: 'this will display OK', nested: { foo: 'this nested field value will not be serialised' } } )

@mderriey
Copy link
Contributor

mderriey commented Apr 16, 2024

I experience the same issue after upgrading to 3.0.0-beta.12, which forces me to stay on beta.11.

For context, we also use winston as a logging framework, so it's the scenario where App Insights uses diagnostic-channel to get access to winston log entries.

@JacksonWeber
Copy link
Contributor

@cuzzlor @mderriey Thank you for your continued usage and testing of the beta! We're moving away from the https://www.npmjs.com/package/diagnostic-channel-publishers solution for collecting Winston logs and moving to the OpenTelemetry instrumentation. While this won't be available in the release of ApplicationInsights 3.0.0, it's undergoing review here Azure/azure-sdk-for-js#29321.

I'll work on addressing the ability to send these kinds of nested logs in the OpenTelemetry transport for Winston https://github.com/open-telemetry/opentelemetry-js-contrib/tree/main/packages/winston-transport. I can alert you both when these changes are completed and the issue of serialization is fully solved (for both our Azure Monitor OpenTelemetry, and Application Insights 3.X SDK offerings).

@mderriey
Copy link
Contributor

mderriey commented Apr 19, 2024

I feel like log collection was the main reason for us to use the App Insights SDK, and not the Azure Monitor OpenTelemetry Distro.

Once the serialisation issue is fixed, we could use the OpenTelemetry transport for winston and get the logs shipped to App Insights through the OTel logs.

Is there a page that lists what the App Insights SDK offers on top of @azure/monitor-opentelemetry? I don't think either of us used the 2.x SDK, and we're familiar with OpenTelemetry, but maybe there are built-in features we don't realise we depend on...

Thanks in advance!

@JacksonWeber
Copy link
Contributor

JacksonWeber commented Apr 19, 2024

@mderriey The official Azure Monitor OpenTelemetry Distro docs are a good resource for the differences as well as the migration guide for those using Application Insights 2.X APIs, but please let me know if you have any further questions regarding comparing feature sets!

Since the Application Insights 3.X SDK is intended to work as a wrapper around the distro, you'd likely be better served by using that product given your familiarity with OpenTelemetry.

@JacksonWeber
Copy link
Contributor

@mderriey @cuzzlor Just wanted to give you both an update that the PR addressing this on the Azure Monitor Distro side has been merged and will be available in the next release Azure/azure-sdk-for-js#29321.

@cuzzlor
Copy link
Author

cuzzlor commented May 8, 2024

Hi @JacksonWeber,

I tried updating to v3.0.1 and updated all the @otel packages to latest, I still get this issue.

Can you please clarify the roadmap for this bug being fixed? It burns a lot of time deploying updates that need to be tested then rolled back.

Thanks!

This is the current state which still shows the same serialisation bug.

    "@opentelemetry/api": "^1.8.0",
    "@opentelemetry/instrumentation": "^0.51.0",
    "@opentelemetry/instrumentation-express": "^0.38.0",
    "@opentelemetry/instrumentation-graphql": "^0.40.0",
    "@opentelemetry/instrumentation-ioredis": "^0.40.0",
    "@opentelemetry/instrumentation-tedious": "^0.10.1",
    "@opentelemetry/instrumentation-undici": "^0.2.0",
    "@opentelemetry/instrumentation-winston": "^0.37.0",
    "@opentelemetry/sdk-trace-node": "^1.24.0",
    "@opentelemetry/semantic-conventions": "^1.24.0",
    "@opentelemetry/winston-transport": "^0.3.0",
    "applicationinsights": "3.0.1",

BTW - After speaking with @mderriey I agree we will swap to using Azure Monitor OpenTelemetry without applicationinsights, once you confirm object serialization is fixed and released. Thanks

@JacksonWeber
Copy link
Contributor

@cuzzlor the fix for object serialization will be coming with the next release of the Azure Monitor OpenTelemetry package that Application Insights 3.X SDK also relies on.

I expect a release of that package this week. Once that release is out, I'll make a release of this repo with the updated Azure Monitor OpenTelemetry package.

I'm glad to hear the AzMon OpenTelemetry package will work for you both! It should be more versatile if you're not dependent on legacy App Insights API support.

@JacksonWeber
Copy link
Contributor

@cuzzlor @mderriey The updated distro (1.5.0) and exporter (beta.23) are now available in the Azure SDK. A release of Application Insights based on these packages will be available soon.

@cuzzlor
Copy link
Author

cuzzlor commented May 11, 2024

Thanks Jackson, will use, appreciate your work and comms thank you. 🙏

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

3 participants