Skip to content

Commit cafe19c

Browse files
comandeo-mongocomandeo-mongo
committed
Respond to code review remarks
1 parent 00c1dc3 commit cafe19c

File tree

5 files changed

+56
-32
lines changed

5 files changed

+56
-32
lines changed

‎source/open-telemetry/open-telemetry.md‎

Lines changed: 19 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -25,7 +25,7 @@ The keywords "MUST", "MUST NOT", "REQUIRED", "SHALL", "SHALL NOT", "SHOULD", "SH
2525

2626
An application that uses the MongoDB driver.
2727

28-
#### Span
28+
**Span**
2929

3030
A Span represents a single operation within a trace. Spans can be nested to form a trace tree. Each trace contains a
3131
root span, which typically describes the entire operation and, optionally, one or more sub-spans for its sub-operations.
@@ -43,21 +43,16 @@ Spans encapsulate:
4343
- A list of timestamped Events
4444
- A Status
4545

46-
#### Tracer
46+
### Implementation Requirements
4747

48-
A Tracer is responsible for creating spans, and using a tracer is the only way to create a span. A Tracer is not
49-
responsible for configuration; this should be the responsibility of the TracerProvider instead.
48+
#### External Dependencies
5049

5150
**OpenTelemetry API and SDK**
5251

5352
OpenTelemetry offers two components for implementing instrumentation – API and SDK. The OpenTelemetry API provides all
5453
the necessary types and method signatures. If there is no OpenTelemetry SDK available at runtime, API methods are
5554
no-ops. OpenTelemetry SDK is an actual implementation of the API. If the SDK is available, API methods do work.
5655

57-
### Implementation Requirements
58-
59-
#### External Dependencies
60-
6156
Drivers MAY add a dependency to the corresponding OpenTelemetry API. This is the recommended way for implementing
6257
OpenTelemetry in libraries. Alternatively, drivers can implement OpenTelemetry support using any suitable tools within
6358
the driver ecosystem. Drivers MUST NOT add a dependency to OpenTelemetry SDK.
@@ -174,9 +169,12 @@ Examples:
174169

175170
##### Exceptions
176171

177-
If the driver operation fails with an exception, drivers MUST record an exception to the current operation span. When
178-
recording an exception, drivers SHOULD add the following attributes to the span, when the content for the attribute if
179-
available:
172+
If the driver operation fails with an exception, drivers MUST record an exception to the current operation span. This
173+
does not relate to exceptions that happen on the server command level (see below). Those exceptions MUST be recorded to
174+
the corresponding command span.
175+
176+
When recording an exception, drivers SHOULD add the following attributes to the span, when the content for the attribute
177+
if available:
180178

181179
- `exception.message`
182180
- `exception.type`
@@ -213,11 +211,13 @@ Spans SHOULD have the following attributes:
213211
| `server.port` | `int64` | Server port number | Required |
214212
| `server.address` | `string` | Name of the database host, or IP address if name is not known | Required |
215213
| `network.transport` | `string` | MUST be 'tcp' or 'unix' depending on the protocol | Required |
216-
| `db.query.summary` | `string` | `command_name database_name.collection_name` | Required |
214+
| `db.query.summary` | `string` | (see explanation below) | Required |
217215
| `db.mongodb.server_connection_id` | `int64` | Server connection id | Required if available |
218216
| `db.mongodb.driver_connection_id` | `int64` | Local connection id | Required if available |
219217
| `db.query.text` | `string` | Database command that was sent to the server. Content should be equivalent to the `document` field of the CommandStartedEvent of the command monitoring. | Conditional |
220218
| `db.mongodb.cursor_id` | `int64` | If a cursor is created or used in the operation | Required if available |
219+
| `db.mongodb.lsid` | `string` | Logical session id | Required is available |
220+
| `db.mongodb.txn_number` | `int64` | Transaction number | Required is available |
221221

222222
Besides the attributes listed in the table above, drivers MAY add other attributes from the
223223
[Semantic Conventions for Databases](https://opentelemetry.io/docs/specs/semconv/registry/attributes/db/) that are
@@ -292,20 +292,20 @@ A common complaint from our support team is that they don't know how to easily g
292292
Some drivers provide debug logging, but others do not. For drivers that do provide it, the log messages produced and the
293293
mechanisms for enabling debug logging are inconsistent.
294294

295-
Although users can implement their own debug logging support via existing driver events (SDAM, APM, etc), this requires
296-
code changes. It is often difficult to quickly implement and deploy such changes in production at the time they are
297-
needed, and to remove the changes afterward. Additionally, there are useful scenarios to log that do not correspond to
298-
existing events. Standardizing on debug log messages that drivers produce and how to enable/configure logging will
299-
provide TSEs, CEs, and MongoDB users an easier way to get debugging information out of our drivers, facilitate support
300-
of drivers for our internal teams, and improve our documentation around troubleshooting.
295+
OpenTelemetry is currently the industry standard for instrumenting, generating, and collecting telemetry data (metrics,
296+
logs, and traces). By instrumenting our drivers natively, we allow our end users to collect traces in a
297+
batteries-included way, with the additional benefits that the tracing is developed and maintained in-house, and conforms
298+
to the open-source standard for tracing. This also ensures that traces generated on the client-side on driver operations
299+
can tie into other traces, thus giving our end users a broader picture of the network hops that a single request might
300+
take.
301301

302302
## Test Plan
303303

304304
See [OpenTelemetry Tests](tests/README.md) for the test plan.
305305

306306
## Covered operations
307307

308-
The OpenTelemetry specification covers the following operations:
308+
The OpenTelemetry specification covers all driver operations including but not limited to the following operations:
309309

310310
| Operation | Test |
311311
| :----------------------- | :----------------------------------------------------------------------------- |

‎source/open-telemetry/tests/transaction/convenient.json‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -120,6 +120,10 @@
120120
"$$type": "string"
121121
},
122122
"db.query.summary": "insert convenient-transaction-tests.test",
123+
"db.mongodb.lsid": {
124+
"$$sessionLsid": "session"
125+
},
126+
"db.mongodb.txn_number": 1,
123127
"db.query.text": {
124128
"$$matchAsDocument": {
125129
"$$matchAsRoot": {
@@ -135,9 +139,6 @@
135139
]
136140
}
137141
}
138-
},
139-
"db.mongodb.lsid": {
140-
"$$sessionLsid": "session"
141142
}
142143
}
143144
}
@@ -164,6 +165,10 @@
164165
},
165166
"db.query.summary": "commitTransaction admin",
166167
"db.command.name": "commitTransaction",
168+
"db.mongodb.lsid": {
169+
"$$sessionLsid": "session"
170+
},
171+
"db.mongodb.txn_number": 1,
167172
"db.query.text": {
168173
"$$matchAsDocument": {
169174
"$$matchAsRoot": {

‎source/open-telemetry/tests/transaction/convenient.yml‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ tests:
7171
server.port: { $$type: [ 'long', 'string' ] }
7272
server.type: { $$type: string }
7373
db.query.summary: insert convenient-transaction-tests.test
74+
db.mongodb.lsid: { $$sessionLsid: *session }
75+
db.mongodb.txn_number: 1
7476
db.query.text:
7577
$$matchAsDocument:
7678
$$matchAsRoot:
@@ -81,7 +83,6 @@ tests:
8183
autocommit: false
8284
documents:
8385
- _id: 1
84-
db.mongodb.lsid: { $$sessionLsid: *session }
8586
- name: commitTransaction admin
8687
attributes:
8788
db.system: mongodb
@@ -96,12 +97,15 @@ tests:
9697
db.collection.name: { $$exists: false }
9798
db.query.summary: commitTransaction admin
9899
db.command.name: commitTransaction
100+
db.mongodb.lsid: { $$sessionLsid: *session }
101+
db.mongodb.txn_number: 1
99102
db.query.text:
100103
$$matchAsDocument:
101104
$$matchAsRoot:
102105
commitTransaction: 1
103106
txnNumber: 1
104107
autocommit: false
108+
105109
- name: find convenient-transaction-tests.test
106110
attributes:
107111
db.system: mongodb

‎source/open-telemetry/tests/transaction/core_api.json‎

Lines changed: 16 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -126,6 +126,10 @@
126126
"$$type": "string"
127127
},
128128
"db.query.summary": "insert transaction-tests.test",
129+
"db.mongodb.lsid": {
130+
"$$sessionLsid": "session0"
131+
},
132+
"db.mongodb.txn_number": 1,
129133
"db.query.text": {
130134
"$$matchAsDocument": {
131135
"$$matchAsRoot": {
@@ -141,9 +145,6 @@
141145
]
142146
}
143147
}
144-
},
145-
"db.mongodb.lsid": {
146-
"$$sessionLsid": "session0"
147148
}
148149
}
149150
}
@@ -170,6 +171,10 @@
170171
},
171172
"db.query.summary": "commitTransaction admin",
172173
"db.command.name": "commitTransaction",
174+
"db.mongodb.lsid": {
175+
"$$sessionLsid": "session0"
176+
},
177+
"db.mongodb.txn_number": 1,
173178
"db.query.text": {
174179
"$$matchAsDocument": {
175180
"$$matchAsRoot": {
@@ -296,6 +301,10 @@
296301
"$$type": "string"
297302
},
298303
"db.query.summary": "insert transaction-tests.test",
304+
"db.mongodb.lsid": {
305+
"$$sessionLsid": "session0"
306+
},
307+
"db.mongodb.txn_number": 1,
299308
"db.query.text": {
300309
"$$matchAsDocument": {
301310
"$$matchAsRoot": {
@@ -311,9 +320,6 @@
311320
]
312321
}
313322
}
314-
},
315-
"db.mongodb.lsid": {
316-
"$$sessionLsid": "session0"
317323
}
318324
}
319325
}
@@ -340,6 +346,10 @@
340346
},
341347
"db.query.summary": "abortTransaction admin",
342348
"db.command.name": "abortTransaction",
349+
"db.mongodb.lsid": {
350+
"$$sessionLsid": "session0"
351+
},
352+
"db.mongodb.txn_number": 1,
343353
"db.query.text": {
344354
"$$matchAsDocument": {
345355
"$$matchAsRoot": {

‎source/open-telemetry/tests/transaction/core_api.yml‎

Lines changed: 8 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -71,6 +71,8 @@ tests:
7171
server.port: { $$type: ['long', 'string'] }
7272
server.type: { $$type: string }
7373
db.query.summary: insert transaction-tests.test
74+
db.mongodb.lsid: { $$sessionLsid: *session0 }
75+
db.mongodb.txn_number: 1
7476
db.query.text:
7577
$$matchAsDocument:
7678
$$matchAsRoot:
@@ -81,7 +83,6 @@ tests:
8183
autocommit: false
8284
documents:
8385
- _id: 1
84-
db.mongodb.lsid: { $$sessionLsid: *session0 }
8586
- name: commitTransaction admin
8687
attributes:
8788
db.system: mongodb
@@ -96,6 +97,8 @@ tests:
9697
db.collection.name: { $$exists: false }
9798
db.query.summary: commitTransaction admin
9899
db.command.name: commitTransaction
100+
db.mongodb.lsid: { $$sessionLsid: *session0 }
101+
db.mongodb.txn_number: 1
99102
db.query.text:
100103
$$matchAsDocument:
101104
$$matchAsRoot:
@@ -164,6 +167,8 @@ tests:
164167
server.port: { $$type: ['long', 'string'] }
165168
server.type: { $$type: string }
166169
db.query.summary: insert transaction-tests.test
170+
db.mongodb.lsid: { $$sessionLsid: *session0 }
171+
db.mongodb.txn_number: 1
167172
db.query.text:
168173
$$matchAsDocument:
169174
$$matchAsRoot:
@@ -174,7 +179,6 @@ tests:
174179
autocommit: false
175180
documents:
176181
- _id: 1
177-
db.mongodb.lsid: { $$sessionLsid: *session0 }
178182
- name: abortTransaction admin
179183
attributes:
180184
db.system: mongodb
@@ -189,6 +193,8 @@ tests:
189193
db.collection.name: { $$exists: false }
190194
db.query.summary: abortTransaction admin
191195
db.command.name: abortTransaction
196+
db.mongodb.lsid: { $$sessionLsid: *session0 }
197+
db.mongodb.txn_number: 1
192198
db.query.text:
193199
$$matchAsDocument:
194200
$$matchAsRoot:
@@ -200,4 +206,3 @@ tests:
200206
- collectionName: test
201207
databaseName: transaction-tests
202208
documents: []
203-

0 commit comments

Comments
 (0)