mongodb / specifications

Specifications related to MongoDB
http://specifications.readthedocs.io/en/latest
Other
389 stars 242 forks source link

DRIVERS-2469 Add durations to connection pool events #1448

Closed stIncMale closed 1 year ago

stIncMale commented 1 year ago

This spec change is implemented in https://github.com/mongodb/mongo-java-driver/pull/1166 for the MongoDB Java Driver.

There are no unified tests for CommandSucceededEvent/CommandFailedEvent.duration (see https://github.com/mongodb/specifications/blob/master/source/command-logging-and-monitoring/command-logging-and-monitoring.rst), similarly, there are no unified tests in this PR except those for logging.

DRIVERS-2469

Please complete the following before merging:

stIncMale commented 1 year ago

A response to https://github.com/mongodb/specifications/pull/1448#pullrequestreview-1570052630 (@baileympearson)

First, we're missing tests for ConnectionCheckoutFailed.duration with the existing tests.

It's here: https://github.com/stIncMale/specifications/blob/DRIVERS-2469/source/connection-monitoring-and-pooling/tests/logging/connection-logging.yml#L216-L224.

Second, the tests currently provide no guarantees about what the fields actually mean. Lastly, the testing strategy relies on logging having been implemented by a driver to test these changes.

Did you consider either trying to add tests to the CMAP unit/integration tests somehow or adding prose tests?

I did. I looked at CommandSucceededEvent/CommandFailedEvent.duration, observed that the only tests for that field is logging tests, and used the same approach, which I also mentioned in the description of the PR. Do you request prose tests for the fields added in this PR?