CXX-2342 add serviceid to events #823
Conversation
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
Codecov Report
@@ Coverage Diff @@
## master #823 +/- ##
==========================================
- Coverage 91.31% 91.28% -0.03%
==========================================
Files 381 381
Lines 22028 22095 +67
==========================================
+ Hits 20114 20170 +56
- Misses 1914 1925 +11
Continue to review full report at Codecov.
|
chardan
force-pushed
the
CXX2342-jfw-add_serviceid
branch
from
6084aed to
cf76bfc
Compare
chardan
changed the title
chardan
force-pushed
the
CXX2342-jfw-add_serviceid
branch
from
cf76bfc to
869a39c
Compare
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
chardan
force-pushed
the
CXX2342-jfw-add_serviceid
branch
from
869a39c to
54abd15
Compare
chardan
force-pushed
the
CXX2342-jfw-add_serviceid
branch
2 times, most recently
from
52a76ab to
4f27798
Compare
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
chardan
force-pushed
the
CXX2342-jfw-add_serviceid
branch
from
4f27798 to
0bc1ae5
Compare
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
src/mongocxx/test/database.cpp
Outdated
| apm_opts.on_command_failed( | ||
| check_service_id<mongocxx::events::command_failed_event>{expect_service_id}); | ||
|
|
||
| // Set up mocking for mongoc_apm_command_started_get_service_id: |
There was a problem hiding this comment.
This applies to all three events. I suggest removing this comment since the comments on L544 and L560 note that mocks are being set.
There was a problem hiding this comment.
Given the high level of confusion that's possible for new-to-the-test-framework folks, I would prefer to leave these here. I agree it's somewhat redundant once it's clear what's going on, but like many other things in this test I think a bit of clarification might very well help the next reader, or one less familiar with the test harnesses. (Although if other reviewers are of the same opinion, I don't have super strong feelings about it.)
There was a problem hiding this comment.
Leaving the comment SGTM. It currently only notes mongoc_apm_command_started_get_service_id being mocked. Since mongoc_apm_command_succeeded_get_service_id and mongoc_apm_command_failed_get_service_id are also mocked, should this be edited to note all three?
There was a problem hiding this comment.
Ah! I see what you're saying! I've gone ahead and revised the comment. Derp!
kevinAlbs
left a comment
kevinAlbs
left a comment
There was a problem hiding this comment.
LGTM with some comment tweaks! Since this is a non-trivial change, can you request review from another team member?
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
Signed-off-by: Jesse Williamson <jesse.williamson@mongodb.com>
4 participants
Add serviceId to CommandFailedEvent, CommandSucceededEvent, and CommandStartedEvent
https://jira.mongodb.org/browse/CXX-2342
Part of CXX-2173 LoadBalancer epic.
Signed-off-by: Jesse Williamson jesse.williamson@mongodb.com