Conversation
…error.type absent on operation span
… name on non-server errors
…n failures aren't retried away
# Conflicts: # source/open-telemetry/open-telemetry.md
There was a problem hiding this comment.
🟡 Changes recommended
Two moderate specification issues leave operation-span behavior and retried command fallback values incorrect or undefined.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Defines consistent OpenTelemetry error.type behavior for MongoDB command spans and adds test coverage.
Changes:
- Specifies command and operation span behavior.
- Adds server and connection-error fixtures.
- Updates the changelog.
File summaries
| File | Description |
|---|---|
source/open-telemetry/tests/operation/error_type.yml |
Adds unified tracing tests for server and connection errors. |
source/open-telemetry/tests/operation/error_type.json |
Adds the generated JSON fixture. |
source/open-telemetry/open-telemetry.md |
Documents error.type; operation-span behavior and fallback semantics require correction and additional coverage. |
Review details
Suppressed comments (2)
source/open-telemetry/open-telemetry.md:378
- The new fixture only exercises failed command spans, so the new MUST NOT requirement on successful commands is not enforced. Existing successful-span expectations also omit this key rather than asserting
$$exists: false, allowing an implementation that emitserror.typeon success to pass. Add a successful command case with an explicit absence assertion.
Drivers MUST NOT set this attribute when the command succeeds. Per the
source/open-telemetry/open-telemetry.md:385
- This is inaccurate for server errors: there
error.typeis the numeric status code whileexception.typeis the exception class, so no duplication occurs and the class is not promoted to the span-levelerror.type. Limit this explanation to the no-status-code fallback.
`error.type` deliberately duplicates the `exception.type` attribute of the exception *event* recorded on the same span
(see Exceptions below). Tracing backends query and aggregate on span attributes, not on the attributes of events nested
within a span, so only `error.type` makes the error class queryable at the span level.
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Balanced
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
There was a problem hiding this comment.
🟡 Changes recommended
The response-status requirement conflicts with connection failures, and the fallback behavior is not adequately tested.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
Previously missed (1) — in code that hasn't changed since the last review.
source/open-telemetry/tests/operation/error_type.yml:129
- This assertion only proves that
error.typeis some string; it does not verify the test's stated fallback to the exception class name or the specified equality withexception.type. An implementation that emits a constant such as"network_error"would pass. Since unified matching cannot compare two observed attributes, add an appended prose test that assertserror.type == exception.type(and, where observable, the raised exception's class name) for this failure.
source/open-telemetry/open-telemetry.md:376
- The value is only a SHOULD even though the table makes the attribute required and this change is intended to give drivers a shared definition. A conforming implementation could emit an unrelated constant string for every failure, defeating interoperable grouping while still satisfying the normative text. Make both derivation branches mandatory; the separate low-cardinality guidance can remain a SHOULD.
This attribute SHOULD match `db.response.status_code` when the command failed with a server error, meaning the server
returned an error code in its response. Otherwise, this attribute SHOULD be the name of the exception class the driver
raises to the application.
- Files reviewed: 3/3 changed files
- Comments generated: 1
- Review effort level: Balanced
… on a non-server error
There was a problem hiding this comment.
🟡 Changes recommended
Moderate issues remain in the specification requirements and test coverage.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (4)
Previously missed (1) — in code that hasn't changed since the last review.
source/open-telemetry/tests/README.md:103
AGENTS.md:121-124requires relative1.numbering for every prose-test step. Using literal numbers makes future insertion and formatting unnecessarily fragile.
source/open-telemetry/open-telemetry.md:376
- The table makes
error.typerequired on every failed command, but theseSHOULDstatements leave its actual value optional, allowing drivers to emit incompatible values despite this PR's shared definition. Make both derivation branches mandatory.
This attribute SHOULD match `db.response.status_code` when the command failed with a server error, meaning the server
returned an error code in its response. Otherwise, this attribute SHOULD be the name of the exception class the driver
raises to the application.
source/open-telemetry/open-telemetry.md:385
- This rationale conflicts with the existing command-span contract below at lines 389–395:
exception.typeis already added as a span attribute (and the fixtures match it there), so it is already queryable at span level. Explain thaterror.typesupplies OpenTelemetry's standard error dimension instead of claiming it introduces span-level queryability.
Tracing backends query and aggregate on span attributes, not on the attributes of events nested within a span.
`error.type` makes the error class queryable at the span level. For a non-server error, it carries the same value as the
`exception.type` attribute of the exception *event* recorded on the same span (see Exceptions below).
source/open-telemetry/open-telemetry.md:255
- No fixture exercises the successful-retry case that motivates this prohibition. The new tests cover a failed operation and a clean success, so an implementation that copies the first failed command's
error.typeonto an operation that later succeeds would still pass. Add a retry-success case asserting no operation-levelerror.typewhile the failed command has one.
Unlike command spans, operation spans MUST NOT have an `error.type` attribute. An operation can succeed through a retry
even when one of its commands failed, so the command-level derivation does not carry over.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
…s fixture case Explains error.type as OpenTelemetry's standard grouping/alerting attribute rather than claiming it is the only span-level error dimension, since exception.type is already a span attribute per the existing Exceptions section. Adds a test covering a failed retried command whose retry succeeds, asserting error.type stays off the operation span.
There was a problem hiding this comment.
🔵 Needs a closer look
Two moderate specification issues must be resolved before approval.
Review details
Suppressed comments (4)
source/open-telemetry/open-telemetry.md:503
- This changelog summary excludes command failures hidden by a successful retry, although the normative text explicitly assigns their command spans an exception-class value. Describe the class associated with the command failure rather than one the driver raises.
command failed with a server error and is otherwise the name of the exception class the driver raises. Specified
source/open-telemetry/open-telemetry.md:376
- The attribute is required on every failed command, but both value-selection rules are only recommendations. That permits conforming drivers to emit different arbitrary values and defeats the cross-driver definition this change introduces; the fixture also requires the exact server-code value. Make both rules
MUSTso the required attribute has an interoperable value.
This attribute SHOULD match `db.response.status_code` when the command failed with a server error, meaning the server
returned an error code in its response. Otherwise, this attribute SHOULD be the name of the exception class associated
with that command's failure, whether or not the operation ultimately raises it to the application: a retry of the same
source/open-telemetry/open-telemetry.md:255
- This blanket prohibition conflicts with OpenTelemetry’s MongoDB span convention, which requires
error.typeif and only if the operation failed. A successful retry should omit the attribute, but an operation that ultimately fails still needs its own final error type; otherwise the public operation span cannot be grouped by the standard error dimension. Base this attribute on the operation’s final outcome and update the failed-operation fixtures accordingly.
Unlike command spans, operation spans MUST NOT have an `error.type` attribute. An operation can succeed through a retry
even when one of its commands failed, so the command-level derivation does not carry over.
source/open-telemetry/tests/README.md:103
- Repository guidance requires relative
1.markers for every prose-test step (AGENTS.md:121-124). Replace these literal numbers so the list remains automatically numbered and future edits do not require manual renumbering.
1. Create a `MongoClient` with tracing enabled and `retryReads` disabled.
2. Configure a `failCommand` fail point on `find` with `closeConnection: true`.
3. Call `find` on a test collection and let it fail.
4. Assert that the command span's `error.type` attribute equals its `exception.type` attribute, and that both equal the
raised exception's fully qualified class name.
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Balanced
…n itself fails The blanket 'operation spans MUST NOT have error.type' prevented backends from grouping failed MongoDB operations by the standard error dimension, even though operation spans already carry exception.type in that case. Operation spans still MUST NOT carry error.type when the operation succeeds despite a failed command, but now SHOULD carry it, matching exception.type, when the operation itself ultimately fails. Updates the two fixtures whose operation span fails to assert this.
…wing the whole table
There was a problem hiding this comment.
🟡 Changes recommended
Resolve the conflicting operation-span requirements and fixture expectations before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (2)
source/open-telemetry/open-telemetry.md:380
- These
SHOULDrequirements allow a conforming driver to choose another value, while the fixture requires the exact server code and the prose test requires exact equality withexception.type. UseMUSTso the normative specification and mandatory tests define the same behavior.
This attribute SHOULD match `db.response.status_code` when the command failed with a server error, meaning the server
returned an error code in its response. Otherwise, this attribute SHOULD be the name of the exception class associated
with that command's failure, whether or not the operation ultimately raises it to the application: a retry of the same
operation may still succeed.
source/open-telemetry/tests/README.md:103
- Use relative
1.markers for every prose-test step, as required byAGENTS.md:123; this prevents later insertions from renumbering steps that drivers may reference.
1. Create a `MongoClient` with tracing enabled and `retryReads` disabled.
2. Configure a `failCommand` fail point on `find` with `closeConnection: true`.
3. Call `find` on a test collection and let it fail.
4. Assert that the command span's `error.type` attribute equals its `exception.type` attribute, and that both equal the
raised exception's fully qualified class name.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
There was a problem hiding this comment.
🟡 Changes recommended
Specification requirements and tests are inconsistent, operation-span equality lacks coverage, and the fixture includes unrelated attributes.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (3)
source/open-telemetry/open-telemetry.md:380
- The value contract is weaker than the fixture: these
SHOULDs permit a driver to emit another value, whileerror_type.ymlrequires the server code and the prose test requires the exception type. Make both mappings mandatory so conforming implementations cannot disagree with the conformance tests.
This attribute SHOULD match `db.response.status_code` when the command failed with a server error, meaning the server
returned an error code in its response. Otherwise, this attribute SHOULD be the name of the exception class associated
with that command's failure, whether or not the operation ultimately raises it to the application: a retry of the same
operation may still succeed.
source/open-telemetry/open-telemetry.md:196
- The operation-span requirement is inconsistent with the new fixture: this table marks
error.typeas conditional and line 257 only says it SHOULD be set, whileerror_type.ymlrequires it on every failed operation span. A driver that legitimately omits this recommended attribute would fail the conformance test. Please either make it required when the operation fails or relax those operation-span expectations.
| `error.type` | `string` | The exception class's name, if the operation fails (see below) | Conditional |
source/open-telemetry/tests/operation/error_type.yml:68
- This focused fixture repeats the full baseline command-span contract in every case.
AGENTS.md:126-129requires new tests to omit fields unrelated to the behavior under test; retaining namespace, network, address, query-summary, and connection attributes makes these tests fail when unrelated telemetry evolves. Reduce all command expectations in this fixture to the attributes needed forerror.typebehavior and span structure.
db.system.name: mongodb
db.namespace: *database0Name
db.collection.name: *collection0Name
db.command.name: find
network.transport: tcp
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Balanced
…n itself fails, and test its equality with exception.type Separates presence (Required if it fails, in the table and prose) from the value's existing SHOULD-derivation, matching the command-level pattern. Extends prose Test 5 to also assert the operation span's error.type equals its exception.type.
There was a problem hiding this comment.
🟡 Changes recommended
Critical specification/test inconsistencies and missing server-error coverage must be resolved before approval.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
source/open-telemetry/tests/README.md:99
- Use relative Markdown numbering (
1.for every step), as required byAGENTS.md:121-124; this keeps later insertions from renumbering prose tests.
1. Create a `MongoClient` with tracing enabled and `retryReads` disabled.
- Files reviewed: 4/4 changed files
- Comments generated: 3
- Review effort level: Balanced
… on a server error's operation span
There was a problem hiding this comment.
🟡 Changes recommended
The new prose tests in source/open-telemetry/tests/README.md require a “fully qualified class name,” which is stricter than the spec’s “exception class name” wording and may impose unintended cross-driver requirements.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Review details
Suppressed comments (1)
source/open-telemetry/tests/README.md:118
- As written, this step requires the raised exception’s fully qualified class name, but the spec only requires the exception class name (and that operation-span
error.typematchesexception.type). Consider relaxing this to “class name” unless the spec is updated to define “fully qualified” for all drivers.
4. Assert that the operation span's `error.type` attribute equals its `exception.type` attribute, and that both equal
the raised exception's fully qualified class name rather than the server error code.
- Files reviewed: 4/4 changed files
- Comments generated: 1
- Review effort level: Lite
…n the error.type prose tests
There was a problem hiding this comment.
🟢 Approval recommended
The spec changes are internally consistent with the accompanying fixtures and changelog update, and no correctness issues were found in the reviewed diffs.
Review details
- Files reviewed: 4/4 changed files
- Comments generated: 0 new
- Review effort level: Lite
| returned an error code in its response. Otherwise, this attribute SHOULD be the name of the exception class associated | ||
| with that command's failure, whether or not the operation ultimately raises it to the application: a retry of the same | ||
| operation may still succeed. |
There was a problem hiding this comment.
Do we have guidance for programming languages where an "exception class" is not always available or practical to determine?
For example, Go has error types, but not all errors require an explicit type, and some error types are part of an error tree containing multiple error types. The guidance below says:
this attribute SHOULD have a low number of distinct values, because tracing backends use it as a dimension for grouping and alerting on failures.
Should drivers for languages without exception classes use an explicitly constrained set of constants?
Also, what should we do for bulk write errors where the error may contain many error codes, one per failed write?
There was a problem hiding this comment.
I am not sure if we can provide generic recommendations for all drivers, this should be decided per language, I believe.
What might be beneficial is to document the list of errors reported (as per https://opentelemetry.io/docs/specs/semconv/registry/attributes/error/#error-type)
There was a problem hiding this comment.
I relaxed the language and the prose tests
There was a problem hiding this comment.
Agreed that a list of example error type values would be helpful. The list in the "semconv" documentation is good.
| | `db.operation.name` | `string` | The name of the driver operation being executed | Required | | ||
| | `db.operation.summary` | `string` | Equivalent to span name | Required | | ||
| | `db.mongodb.cursor_id` | `int64` | If a cursor is created or used in the operation (see below) | Conditional | | ||
| | `error.type` | `string` | The exception class's name, if the operation fails (see below) | Required if it fails | |
There was a problem hiding this comment.
We do not list exception.* attributes here, only in the "Exceptions" section. I think we should either list all attributes in the table, or move all of them to a dedicated section. Both ways are fine by me.
| | `db.collection.name` | `string` | The collection being accessed within the database stated in `db.namespace` | Required if available | | ||
| | `db.command.name` | `string` | The name of the server command being executed | Required | | ||
| | `db.response.status_code` | `string` | MongoDB error code represented as a string. This attribute should be added only if an error happens. | Required if an error happens | | ||
| | `error.type` | `string` | See [error.type](#errortype) below. | Required if an error happens | |
There was a problem hiding this comment.
We do not list exception.* attributes here, only in the "Exceptions" section. I think we should either list all attributes in the table, or move all of them to a dedicated section. Both ways are fine by me.
| returned an error code in its response. Otherwise, this attribute SHOULD be the name of the exception class associated | ||
| with that command's failure, whether or not the operation ultimately raises it to the application: a retry of the same | ||
| operation may still succeed. |
There was a problem hiding this comment.
I am not sure if we can provide generic recommendations for all drivers, this should be decided per language, I believe.
What might be beneficial is to document the list of errors reported (as per https://opentelemetry.io/docs/specs/semconv/registry/attributes/error/#error-type)
| 5. Assert that both the `getMore` operation span and the `getMore` command span have a `db.mongodb.cursor_id` attribute | ||
| whose value equals the cursor id recorded in step 3. | ||
|
|
||
| *Test 5: `error.type` equals `exception.type` for a non-server error* |
There was a problem hiding this comment.
In the spec we say about error.type "Its value SHOULD be the name of the exception class raised to the
application, the same value as the operation span's exception.type attribute above.", so this is not a requirement. Do we need a test in this case?
# Conflicts: # source/open-telemetry/open-telemetry.md # source/open-telemetry/tests/README.md
…n the error.type prose tests
nhachicha
left a comment
nhachicha
left a comment
There was a problem hiding this comment.
Looks good minor comments 👍
|
|
||
| Drivers MUST NOT set this attribute when the command succeeds. Per the | ||
| [OpenTelemetry semantic conventions for `error.type`](https://opentelemetry.io/docs/specs/semconv/registry/attributes/error/#error-type), | ||
| this attribute SHOULD have a low number of distinct values, because tracing backends use it as a dimension for grouping |
There was a problem hiding this comment.
low cardinality is the term used by the OpenTelemetry semantic conventions
| this attribute SHOULD have a low number of distinct values, because tracing backends use it as a dimension for grouping | |
| this attribute SHOULD have a low cardinality, because tracing backends use it as a dimension for grouping |
|
|
||
| - 2026-08-19: Specified the `error.type` attribute on command spans, which matches `db.response.status_code` when the | ||
| command failed with a server error and is otherwise the name of the exception class associated with that command's | ||
| failure. Specified that drivers MUST NOT set it when the command succeeds, that it SHOULD have a low number of |
|
|
||
| ###### error.type | ||
|
|
||
| This attribute SHOULD match `db.response.status_code` when the command failed with a server error, meaning the server |
There was a problem hiding this comment.
Nothing here requires drivers to set error.type on a failed command (only value SHOULDs and a MUST NOT on success), yet the tests assert its presence. Suggest:
| This attribute SHOULD match `db.response.status_code` when the command failed with a server error, meaning the server | |
| Drivers MUST add this attribute to the command span when the command fails. This attribute SHOULD match `db.response.status_code` when the command failed with a server error, meaning the server |
| | `db.collection.name` | `string` | The collection being accessed within the database stated in `db.namespace` | Required if available | | ||
| | `db.operation.name` | `string` | The name of the driver operation being executed | Required | | ||
| | `db.operation.summary` | `string` | Equivalent to span name | Required | | ||
| | `db.mongodb.cursor_id` | `int64` | If a cursor is created or used in the operation (see below) | Conditional | |
There was a problem hiding this comment.
Both attribute tables should gain an error.type row so the tables stay the complete inventory of span attributes.
Suggestion:
| `error.type` | `string` | (see [error.type](#errortype-1) below) | Required if the operation fails |
| | `db.namespace` | `string` | The database name | Required if available | | ||
| | `db.collection.name` | `string` | The collection being accessed within the database stated in `db.namespace` | Required if available | | ||
| | `db.command.name` | `string` | The name of the server command being executed | Required | | ||
| | `db.response.status_code` | `string` | MongoDB error code represented as a string. This attribute should be added only if an error happens. | Required if an error happens | |
There was a problem hiding this comment.
| `error.type` | `string` | (see [error.type](#errortype) below) | Required if the command fails |
| 5. Assert that both the `getMore` operation span and the `getMore` command span have a `db.mongodb.cursor_id` attribute | ||
| whose value equals the cursor id recorded in step 3. | ||
|
|
||
| *Test 5: `error.type` is present for a non-server error* |
There was a problem hiding this comment.
The UTR fixture already asserts error.type: {$$type: string} here, so these presence-only prose tests can't fail on a driver that passes the fixture. Either:
1 - strengthen them (Test 5: assert the exact exception class name; Test 6: assert the operation span's error.type differs from the command span's, the comparison the intro promises)
2- drop both.
WDYT?
DRIVERS-3617
Please complete the following before merging:
Summary
error.typeis the OpenTelemetry semantic-convention attribute that tracing backends recognize for grouping and alerting on failures, but the command span spec defined onlydb.response.status_code.exception.typeisn't part of that convention, even though drivers also add it to the span, so drivers that emit it anyway have no shared definition of its value.Changes
error.typeto the Command Span Attributes table as a string, required if an error happens.db.response.status_codefor server errors, otherwise the name of the exception class associated with that command's failure (which may never reach the application, if a retry saves the operation).exception.type, when the operation itself ultimately fails.error_type.yml) covering: a server error; a closed connection falling back to the class name; success; a failed command whose retry succeeds; and a failed retry whose exception never reaches the application.open-telemetry.md.Testing
make -C source, then confirmed the regenerated JSON left no diff.pre-commit run --all-files: clean.mkdocs build --strict: clean.schema-1.27.jsonwithajv-cli: valid.