Skip to content

DOCSP-38042: CMAP Tracking Example #891

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 4 commits into from
May 30, 2024

Conversation

mcmorisi
Copy link
Collaborator

@mcmorisi mcmorisi commented May 29, 2024

Pull Request Info

Paraphrased from requester: "The context here is to showcase a practical example of what CMAP monitoring could be used for – the existing "Event Subscription Example" shows how to subscribe, but not really why you'd bother in the first place."

Code sample provided by requester and tested by me (with comments added).

PR Reviewing Guidelines

JIRA - https://jira.mongodb.org/browse/DOCSP-38042
Staging - https://preview-mongodbmcmorisi.gatsbyjs.io/node/DOCSP-38042-connection-tracking/fundamentals/monitoring/connection-monitoring/

Self-Review Checklist

  • Is this free of any warnings or errors in the RST?
  • Did you run a spell-check?
  • Did you run a grammar-check?

@mcmorisi mcmorisi changed the title Docsp 38042 connection tracking DOCSP-38042: CMAP Tracking Example May 29, 2024
Copy link
Collaborator

@norareidy norareidy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Just a few small things!

checkedOut = NaN;
}

// Increases count of active connection pool threads when connectionCheckedIn event is triggered
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I: since this line calls the onCheckin() function, which decreases the checkedOut count, it seems a little misleading to say that this is increasing a count of active connection pool threads. Maybe you could reword to:

Suggested change
// Increases count of active connection pool threads when connectionCheckedIn event is triggered
// Decreases count of connections checked out of the pool when connectionCheckedIn event is triggered

// Increases count of active connection pool threads when connectionCheckedIn event is triggered
client.on('connectionCheckedIn', onCheckin);

// Decreases count of active connection pool threads when connectionCheckedOut event is triggered
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I: similar comment as above

Suggested change
// Decreases count of active connection pool threads when connectionCheckedOut event is triggered
// Increases count of connections checked out of the pool when connectionCheckedOut event is triggered

Comment on lines 44 to 46
the behavior of your application's connection pool. The following example
demonstrates a function that uses connection pool monitoring events to return a count
of checked-out connections in the pool.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S: I think this sentence could be a little more concise, and it also can end with a colon. Maybe something like:

Suggested change
the behavior of your application's connection pool. The following example
demonstrates a function that uses connection pool monitoring events to return a count
of checked-out connections in the pool.
the behavior of your application's connection pool. The following example uses connection pool monitoring events to return a count
of checked-out connections in the pool:

@@ -29,8 +29,8 @@ application run faster.
The following sections demonstrate how to record connection pool events in your
application or want to explore the information provided in these events.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

S: out of scope for this ticket so not a necessary change, but this sentence is confusing. I think this wording makes more sense:

Suggested change
application or want to explore the information provided in these events.
application and explore the information provided in these events.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good eye!

Copy link
Collaborator

@norareidy norareidy left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lgtm!

@mcmorisi mcmorisi merged commit 1859354 into mongodb:master May 30, 2024
@mcmorisi mcmorisi deleted the DOCSP-38042-connection-tracking branch May 30, 2024 13:21
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
mcmorisi added a commit that referenced this pull request May 30, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants