feat(sdk-logs,api-logs): support exceptions in Logger API

[iblancasa]

Which problem is this PR solving?

Fixes #6379

Short description of the changes

Implement Exceptions support from spec:

• Logs API spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/api.md#emit-a-logrecord
• Logs SDK spec: https://github.com/open-telemetry/opentelemetry-specification/blob/main/specification/logs/sdk.md

Type of change

Please delete options that are not relevant.

• New feature (non-breaking change which adds functionality)

How Has This Been Tested?

• Created some automated tests

Checklist:

• Followed the style guidelines of this project
• Unit tests have been added

[codecov]

Codecov Report

✅ All modified and coverable lines are covered by tests.
✅ Project coverage is 95.51%. Comparing base (bfd06e6) to head (9222bf1).
⚠️ Report is 6 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #6385      +/-   ##
==========================================
+ Coverage   95.50%   95.51%   +0.01%     
==========================================
  Files         365      365              
  Lines       11617    11646      +29     
  Branches     2681     2694      +13     
==========================================
+ Hits        11095    11124      +29     
  Misses        522      522              

Files with missing lines	Coverage Δ
...erimental/packages/api-logs/src/types/LogRecord.ts	100.00% <ø> (ø)
...xperimental/packages/sdk-logs/src/LogRecordImpl.ts	99.30% <100.00%> (+0.17%)	⬆️

🚀 New features to boost your workflow:

• ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• 📦 JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[pichlermarc]

please add @experimental annotations to each of the added types/methods - we're currently in the process of stabilizing the package, so experimental APIs should be marked accordingly.

[pichlermarc]

thanks for addressing the comments :)
a few more suggestions then this should be good to merge. :)

[trentm]

[I'm not sure if this was discussed already.]

Whether to use "exception" or "error"? In JavaScript-land my impression is that "error" is the term for the thing we are talking about here. In Node.js, "exception" tends to lead one to Node's "uncaughtException" event. However, both terms are fine. MDN's https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error somewhat interchangeably uses both.

Given that (a) OTel specs use "exception", (b) adding this API is part of deprecating Span#recordException, and (c) there is some overloading / possible confusion with the "error" log/severity level, I'm fine with exception. I'd be fine with the property being called "error" as well.

[pichlermarc]

Personally, I'd gravitate towards error, but the confusion with the severity level is a good point.
Let's keep exception. 👍

[trentm]

The implementation here is transcribing what Span#recordException() currently does, which is fine.

for another PR/issue to discuss

I don't think this PR needs to change. I'm throwing out some ideas, because I've been around logging libs for too long.

There is a part of me that wants to do more here, e.g.:

• handle a thrown number (not really a high priority, but for completeness :)
• handle a exception.cause now that Error can take a cause option (since Node.js 16.9, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error)
• handle other properties added to the Error instance. Specifically setting ATTR_EXCEPTION_TYPE to exception.code || exception.name feels slightly like a kludge. However, a better answer would likely require adding a new exception.* attribute to semconv, which might be an uphill battle.

err.cause

Inspired by Pino's serializers for Errors (https://github.com/pinojs/pino-std-serializers?tab=readme-ov-file#serializers) some options:

1. Append the err.cause.stack, if any, to the ATTR_EXCEPTION_STACKTRACE (as Pino's err serializer does). I wonder if OTel Java is already doing this? Yes, I think it does, it is using Throwable.printStackTrace: https://github.com/open-telemetry/opentelemetry-java/blob/main/sdk/common/src/main/java/io/opentelemetry/sdk/common/internal/DefaultExceptionAttributeResolver.java#L45-L58 https://docs.oracle.com/javase/8/docs/api/java/lang/Throwable.html#printStackTrace--
2. Fight the power! and push for a deeply nested ATTR_EXCEPTION semconv property that can have nested cause values. :) I'm kidding. This isn't going to happen. Possibly a ATTR_EXCEPTION_CAUSE_* set of attributes could be added for one level of nesting, but I'm not sure that is valuable.

other exception properties

For example:

> assert.strictEqual(1, 2)
Uncaught AssertionError [ERR_ASSERTION]: Expected values to be strictly equal:

1 !== 2

    at REPL126:1:8
    at ContextifyScript.runInThisContext (node:vm:137:12)
    at REPLServer.defaultEval (node:repl:562:24)
    at bound (node:domain:433:15)
    at REPLServer.runBound [as eval] (node:domain:444:12)
    at REPLServer.onLine (node:repl:886:12)
    at REPLServer.emit (node:events:520:35)
    at REPLServer.emit (node:domain:489:12)
    at [_onLine] [as _onLine] (node:internal/readline/interface:465:12)
    at [_line] [as _line] (node:internal/readline/interface:953:18) {
  generatedMessage: true,
  code: 'ERR_ASSERTION',
  actual: 1,
  expected: 2,
  operator: 'strictEqual',
  diff: 'simple'
}

[iblancasa]

• handle a thrown number (not really a high priority, but for completeness :)

Adding.

• handle a exception.cause now that Error can take a cause option (since Node.js 16.9, https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Error/Error)

Interesting. Maybe we can propose this to the semantic conventions since it makes sense to me. What do you think?

• handle other properties added to the Error instance. Specifically setting ATTR_EXCEPTION_TYPE to exception.code || exception.name feels slightly like a kludge. However, a better answer would likely require adding a new exception.* attribute to semconv, which might be an uphill battle.

Maybe. I can draft a PR on top of this one and create a proposal from semcon.

Your suggestions sound very reasonable to me and I guess those also make sense for other languages too.

[trentm]

I think it would be worth looking at the discussions that led to the exception.* attributes in the spec/semconv to see if cause and/or code (or other exception attributes) were discussed.

I think it would be worthwhile to have a follow-up issue to consider adding exception.cause?.stack to the ATTR_EXCEPTION_STACKTRACE attribute.

[iblancasa]

Yes. Between today and tomorrow I will do some research and create one proposal if it doesn't exist yet. I will mention you and crosslink this PR to provide some context.

[pichlermarc]

Approving since this looks good to me - let's give some time for @trentm to have another look, then we can merge.

[trentm]

LGTM. Thanks!

[trentm]

I think it would be worth looking at the discussions that led to the exception.* attributes in the spec/semconv to see if cause and/or code (or other exception attributes) were discussed.

I think it would be worthwhile to have a follow-up issue to consider adding exception.cause?.stack to the ATTR_EXCEPTION_STACKTRACE attribute.