open-telemetry / opentelemetry-js

OpenTelemetry JavaScript Client
https://opentelemetry.io
Apache License 2.0
2.75k stars 808 forks source link

[sdk-trace] no type validation in places where it is common to pass caught `Error`s #4998

Closed pichlermarc closed 1 month ago

pichlermarc commented 1 month ago

What happened?

Description

When calling span.setStatus({ code: SpanStatusCode.ERROR, message: message}) it could be common to pass a caught Error as the message, like so:

try {
  myFunction(); // throws error
} catch (e) { // if typescript strict mode is NOT enabled, `e` is of type `any`
      span.setStatus({
        code: SpanStatusCode.ERROR,
        message: e, //  this expects a `string`, but receives `Error`, which TypeScript treats as `any`, so this does not cause a compile error.
      });
}

Expected Result

message is dropped as being not a string, a diag.warn is logged.

Actual Result

message which should be a string gets passed all though the SDK, to the exporter, and eventually gets JSON.stringifyd into an empty object, the OTel collector then rejects the whole trace export message with a 400 Bad Request.

Additional Details

This is happening in renovate bot where they catch an error and pass it directly as a message in setStatus: https://github.com/renovatebot/renovate/blob/5ca09edcbf454fc1b1ae9272ab240cd03d8d2e75/lib/instrumentation/index.ts#L145-L153