-
Notifications
You must be signed in to change notification settings - Fork 141
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]" #1169
Comments
I spent some time debugging this today, and believe I might have found something. First, when debugging, I could consistently see those messages in the debug windows: ApplicationInsights:Invalid attribute value set for key: parameters
ApplicationInsights:Invalid attribute value set for key: request Then, going all the way down to the Here's a sample of a request body, somewhat redacted: [
{
"ver": 1,
"name": "Microsoft.ApplicationInsights.Message",
"time": "2023-07-07T14:58:07.162Z",
"sampleRate": 100,
"iKey": "<redacted>",
"tags": {
"ai.device.osVersion": "Windows_NT 10.0.22621",
"ai.internal.sdkVersion": "node18:otel1.14.0:dst3.0.0-beta.6",
"ai.cloud.role": "<redacted>",
"ai.cloud.roleInstance": "DESKTOP-DIT65QM",
"ai.operation.id": "c7ff8b12cf39775c3af9586494c544c2",
"ai.operation.parentId": "e87b299aa6b3e33b"
},
"data": {
"baseType": "MessageData",
"baseData": {
"ver": 2,
"message": "GraphQL operation",
"severityLevel": "Information",
"properties": {
"request": { "requestId": "998d84d6-65fa-4f91-8e2b-03c3980a03e1" },
"tenantAlias": "Local",
"service": "<redacted>",
"operationName": "Sessions",
"query": "query Sessions($offset: NonNegativeInt!, $limit: PositiveInt!) {\r\n findEvents(offset: $offset, limit: $limit) {\r\n id\r\n agenda {\r\n items {\r\n ... on Session {\r\n id\r\n name\r\n }\r\n }\r\n }\r\n }\r\n}",
"variables": { "offset": 100, "limit": 3 },
"duration": 253,
"trace_id": "c7ff8b12cf39775c3af9586494c544c2",
"span_id": "e87b299aa6b3e33b",
"trace_flags": "01"
},
"measurements": {}
}
}
}
] and the HTTP request options: {
"hostname": "australiaeast-0.in.applicationinsights.azure.com",
"path": "/v2.1/track",
"port": "",
"method": "POST",
"headers": {
"Content-Type": "application/json",
"Accept": "application/json",
"Accept-Encoding": "gzip,deflate",
"User-Agent": "azsdk-js-monitor-opentelemetry-exporter/1.0.0-beta.14 core-rest-pipeline/1.11.0 Node/v18.16.1 OS/(x64-Windows_NT-10.0.22621)",
"x-ms-client-request-id": "14a5cd65-2e07-4db7-ac99-68453532b250",
"Content-Length": "2952",
"traceparent": "00-c7ff8b12cf39775c3af9586494c544c2-ad9ef54e2e76a6bb-01"
}
} And the associated trace in the portal (notice the operation ID highlighted in green that shows it's the same telemetry item) It's as if the service was choking on the nested objects in the Let me know if that helps. |
Hi @hectorhdzg, could you please have a look at this when you have some time and let me know if I can provide you with more information to help diagnose this? Cheers 🙏 |
@mderriey I just wanted to let you know I was able to reproduce your issue, even with a very basic Winston logging event. I'll test the other console instrumentations as well and look into what might be causing this. Thank you for raising the issue! |
@JacksonWeber do you have any status update on this issue? |
@cuzzlor I'be had my head down in some high priority work for the SDK, but I can dedicate some time back to this issue tonight. Thanks for bringing it up! |
) ### Packages impacted by this PR @azure/monitor-opentelemetry-exporter ### Issues associated with this PR microsoft/ApplicationInsights-node.js#1169 ### Describe the problem that is addressed by this PR Fixes an issue with serializing legacy Application Insights logs with nested objects. ### Are there test cases added in this PR? _(If not, why?)_ Yes, added in the `logUtils.tests` file. ### Checklists - [x] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [x] Added a changelog (if necessary)
Thanks heaps @JacksonWeber. I suspect this was on your list, but will you be able to leave us a note here when a new beta that uses Cheers. |
@mderriey Absolutely, I'll make sure to leave you a note here as soon as it's available! |
@mderriey @cuzzlor The issue should be resolved as of 3.0.0 beta.8 https://github.com/microsoft/ApplicationInsights-node.js/releases/tag/3.0.0-beta.8 |
Thanks @JacksonWeber Unfortunately, it doesn't seem to be the case. Setup code now looks as follows: import { ExpressInstrumentation } from '@opentelemetry/instrumentation-express'
import { HttpInstrumentation } from '@opentelemetry/instrumentation-http'
import { Resource } from '@opentelemetry/resources'
import { SemanticResourceAttributes } from '@opentelemetry/semantic-conventions'
import { TediousInstrumentation } from '@opentelemetry/instrumentation-tedious'
import { DataloaderInstrumentation } from '@opentelemetry/instrumentation-dataloader'
import { RedisInstrumentation } from '@opentelemetry/instrumentation-redis-4'
import { SetTenantAliasAttributeSpanProcessor, tenantAliasAttributeName } from './tracing/set-tenant-alias-attribute-span-processor'
import { IncomingMessage } from 'node:http'
import { getTenantAlias } from './util/get-tenant-alias'
import { TruncateDbStatementAttributeSpanProcessor } from './tracing/truncate-db-statement-attribute-span-processor'
import { useAzureMonitor } from 'applicationinsights'
import type { ProxyTracerProvider } from '@opentelemetry/api'
import { trace } from '@opentelemetry/api'
import type { NodeTracerProvider } from '@opentelemetry/sdk-trace-node'
import { registerInstrumentations } from '@opentelemetry/instrumentation'
// Default attributes set on the OpenTelemetry traces
const resource = Resource.EMPTY
resource.attributes[SemanticResourceAttributes.SERVICE_NAME] = '<redacted>'
resource.attributes[SemanticResourceAttributes.SERVICE_NAMESPACE] = '<redacted>'
useAzureMonitor({
resource,
instrumentationOptions: {
http: { enabled: false },
},
logInstrumentationOptions: {
winston: { enabled: true },
},
})
const tracerProvider = (trace.getTracerProvider() as ProxyTracerProvider).getDelegate() as NodeTracerProvider
tracerProvider.addSpanProcessor(new SetTenantAliasAttributeSpanProcessor())
tracerProvider.addSpanProcessor(new TruncateDbStatementAttributeSpanProcessor())
registerInstrumentations({
tracerProvider,
instrumentations: [
new HttpInstrumentation({
// We need to manually set the attribute for requests spans because the OTel context
// with the tenant alias is not available when this hook is invoked.
applyCustomAttributesOnSpan(span, request) {
if (request instanceof IncomingMessage) {
const tenantAlias = getTenantAlias(request)
if (tenantAlias) {
span.setAttribute(tenantAliasAttributeName, tenantAlias)
}
}
},
}),
new ExpressInstrumentation(),
new TediousInstrumentation(),
new DataloaderInstrumentation(),
new RedisInstrumentation(),
],
}) |
@mderriey Taking a look this morning |
) ### Packages impacted by this PR @azure/monitor-opentelemetry-exporter ### Issues associated with this PR microsoft/ApplicationInsights-node.js#1169 ### Describe the problem that is addressed by this PR Fixes an issue with serializing legacy Application Insights logs with nested objects. ### Are there test cases added in this PR? _(If not, why?)_ Yes, added in the `logUtils.tests` file. ### Checklists - [x] Added impacted package name to the issue description - [ ] Does this PR needs any fixes in the SDK Generator?** _(If so, create an Issue in the [Autorest/typescript](https://github.com/Azure/autorest.typescript) repository and link it here)_ - [x] Added a changelog (if necessary)
@mderriey Dug into this today and it looks like the issue is with how OpenTelemetry defines the |
Hmm, OK, thanks for looking into this. I managed to get it working by updating the private _telemetryToLogRecord(
telemetry: Contracts.Telemetry,
baseType: string,
baseData: MonitorDomain
): LogRecord {
try {
- const attributes: Attributes = {
- ...telemetry.properties,
- };
+ const attributes: Attributes = {};
+ if (telemetry.properties) {
+ for (const [key, value] of Object.entries(telemetry.properties)) {
+ attributes[key] = typeof value === 'object'
+ ? JSON.stringify(value)
+ : value
+ }
+ }
const record: LogRecord = { attributes: attributes, body: Util.getInstance().stringify(baseData) };
record.attributes["_MS.baseType"] = baseType;
return record;
}
catch (err) {
Logger.getInstance().warn("Failed to convert telemetry event to Log Record.", err);
}
} It then shows up properly in the portal I did it there because it's one the place where we construct an OpenTelemetry Is this something that we could imagine to be taken in the repo, or does this change have side effects that could impact other areas in the library? Happy to open a PR if that helps you. |
Apologies, @JacksonWeber, I didn't tag you on the comment above, could you please have a look and let us know how that sounds to you? |
@mderriey I'll look through to make sure this won't have any adverse effects elsewhere in the SDK, but I'll take a look this morning, thanks again! |
@mderriey Did some testing and it looks good. Feel free to create a PR if you'd like! |
Hi there 👋
I hope you're well.
Issue
We've upgraded from
3.0.0-beta.5
to3.0.0-beta.6
recently and we noticed that nested objects get serialised as "[object Object]".See the screenshot below.
For reference, we deployed the package upgrade sometime during the 3rd of July.
No traces before the 3rd of July show
customDimensions.variables
with a value of[object Object]
Setup
We don't interact with the SDK or the trace handler directly, all our logging is done through winston, and we have the appropriate log instrumentation enabled.
Here's the whole config, in case it helps:
The logging code is standard winston and looks like this:
Notes
Possibly related to #1168, although in that case it looks like they interact with the SDK directly and had to make some modifications to their code.
The text was updated successfully, but these errors were encountered: