DOCSP-43082: Cluster monitoring #63

norareidy · 2024-10-01T15:59:16Z

Pull Request Info

JIRA - https://jira.mongodb.org/browse/DOCSP-43082
Staging - https://deploy-preview-63--docs-c.netlify.app/monitoring/cluster-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?
Are all the links working?
Are the facets and meta keywords accurate?

netlify · 2024-10-01T15:59:31Z

✅ Deploy Preview for docs-c ready!

Name	Link
🔨 Latest commit	`aae7635`
🔍 Latest deploy log	https://app.netlify.com/sites/docs-c/deploys/66fdab9fbfca460008d8f23d
😎 Deploy Preview	https://deploy-preview-63--docs-c.netlify.app
📱 Preview on mobile	Toggle QR Code... Use your smartphone camera to open QR code link.

To edit notification comments on pull requests, go to your Netlify site configuration.

mcmorisi

LGTM with some small things!

mcmorisi · 2024-10-01T17:15:42Z

source/monitoring/cluster-monitoring.txt

+actions:
+
+- Defines a ``server_opening()`` event handler function, or callback
+- Creates a ``mongoc_apm_callbacks_t`` to store callbacks


Suggested change

- Creates a ``mongoc_apm_callbacks_t`` to store callbacks

- Creates a ``mongoc_apm_callbacks_t`` instance to store callbacks

Is it accurate to use "instance" with C? "Structure" might be a better word

source/includes/monitoring/sdam.c

eramongodb · 2024-10-01T20:26:24Z

source/monitoring/cluster-monitoring.txt

+in your application. To subscribe to an event, create a value of
+type ``mongoc_apm_callbacks_t`` and create an event callback function
+for the event you want to subscribe to. Use the callback's corresponding
+setter method to set the callback on the ``mongoc_apm_callbacks_t`` and 
+pass this type to the ``mongoc_client_set_apm_callbacks()`` function.


Recommended rewording:

To subscribe to an event, define a callback function to handle each event type you want to subscribe to. Use a mongoc_apm_callbacks_t object to register the list of APM callbacks with a client object using mongoc_client_set_apm_callbacks().

There is a lot of overlap with the actual code example below. Wordy details can be delegated to the code example.

Also recommend referring to these user-defined functions as "APM callbacks" or "APM callback functions" for consistency with C Driver API documentation.

eramongodb · 2024-10-01T20:29:35Z

source/monitoring/cluster-monitoring.txt

+This code monitors server opening events by performing the following
+actions:
+
+- Defines a ``server_opening()`` event handler function, or callback


Suggested change

- Defines a ``server_opening()`` event handler function, or callback

- Defines a ``server_opening()`` APM callback function

For consistent terminology.

eramongodb · 2024-10-01T20:29:50Z

source/monitoring/cluster-monitoring.txt

+actions:
+
+- Defines a ``server_opening()`` event handler function, or callback
+- Creates a ``mongoc_apm_callbacks_t`` structure to store callbacks


Suggested change

- Creates a ``mongoc_apm_callbacks_t`` structure to store callbacks

- Creates a ``mongoc_apm_callbacks_t`` object to store callbacks

eramongodb · 2024-10-01T20:30:32Z

source/monitoring/cluster-monitoring.txt

+- Calls the ``mongoc_apm_set_server_opening_cb()`` function, which instructs
+  the ``mongoc_apm_callbacks_t`` type to store a pointer to the ``server_opening()``
+  callback


Suggested change

- Calls the ``mongoc_apm_set_server_opening_cb()`` function, which instructs

the ``mongoc_apm_callbacks_t`` type to store a pointer to the ``server_opening()``

callback

- Calls the ``mongoc_apm_set_server_opening_cb()`` function, which stores a pointer to the provided APM callback function in the ``mongoc_apm_callbacks_t`` object

eramongodb · 2024-10-01T20:44:01Z

source/monitoring/cluster-monitoring.txt

+  the ``mongoc_apm_callbacks_t`` type to store a pointer to the ``server_opening()``
+  callback
+- Calls the ``mongoc_client_set_apm_callbacks()`` function, which registers
+  the callback with the client


Suggested change

the callback with the client

the list of callbacks in the ``mongoc_apm_callbacks_t`` object with the client

There's only one callback in this code example (server_opening()) rather than a list so I reworded slightly

eramongodb · 2024-10-01T20:44:53Z

source/includes/monitoring/sdam.c

+   stats_t *context;
+
+   context = (stats_t *) mongoc_apm_server_opening_get_context (event);
+   context->server_opening_events++;


Suggested change

stats_t *context;

context = (stats_t *) mongoc_apm_server_opening_get_context (event);

context->server_opening_events++;

stats_t *stats = (stats_t *) mongoc_apm_server_opening_get_context (event);

stats->server_opening_events += 1;

eramongodb · 2024-10-01T20:48:54Z

source/includes/monitoring/sdam.c

+   mongoc_client_t *client;
+   mongoc_apm_callbacks_t *cbs
+   stats_t stats = {0};


Suggested change

mongoc_client_t *client;

mongoc_apm_callbacks_t *cbs

stats_t stats = {0};

Avoid C89/C90 style declarations.

Also recommend including use of the stats object.

int main (void) { mongoc_init (); stats_t stats = {0}; mongoc_client_t *client = mongoc_client_new ("<connection string URI>"); { mongoc_apm_callbacks_t *cbs = mongoc_apm_callbacks_new (); mongoc_apm_set_server_opening_cb (cbs, server_opening); mongoc_client_set_apm_callbacks (client, cbs, &stats); mongoc_apm_callbacks_destroy (cbs); } // Perform database operations. mongoc_client_destroy (client); printf ("Observed %d server opening events\n", stats.server_opening_events); mongoc_cleanup (); return EXIT_SUCCESS; }

eramongodb · 2024-10-01T20:49:05Z

source/includes/monitoring/sdam.c

+   mongoc_cleanup ();
+
+   return EXIT_SUCCESS;
+}


Suggested change

}

}

Missing EOF newline.

eramongodb

Minor feedback remaining; otherwise, LGTM.

eramongodb · 2024-10-02T18:33:35Z

source/monitoring/cluster-monitoring.txt

+   Server opening on ac-rmuag0v-shard-00-00.gh0qg50.mongodb.net:27017
+   Server opening on ac-rmuag0v-shard-00-01.gh0qg50.mongodb.net:27017
+   Server opening on ac-rmuag0v-shard-00-02.gh0qg50.mongodb.net:27017


These examples seem a bit noisy / unnecessarily specific ("ac-rmuag0v"? "gh0qg50"?). Can they be simplified?

eramongodb · 2024-10-02T18:34:12Z

source/monitoring/cluster-monitoring.txt

+------------------
+
+You can subscribe to SDAM events by defining the corresponding
+event callback function. The following table provides the name of


Suggested change

event callback function. The following table provides the name of

APM callback function. The following table provides the name of

Consistency.

DOCSP-43082: Cluster monitoring

145b7fe

norareidy added 2 commits October 1, 2024 12:10

edits

62dfd6c

fix link

0af3018

mcmorisi approved these changes Oct 1, 2024

View reviewed changes

norareidy added 2 commits October 1, 2024 13:26

MM feedback

7e94278

wording

dcb0c6d

norareidy requested a review from kevinAlbs October 1, 2024 17:45

kevinAlbs requested review from eramongodb and removed request for kevinAlbs October 1, 2024 20:04

eramongodb suggested changes Oct 1, 2024

View reviewed changes

EC feedback

d7180a3

norareidy requested a review from eramongodb October 2, 2024 13:43

eramongodb mentioned this pull request Oct 2, 2024

Fix and update references to Markdown MongoDB specifications mongodb/mongo-c-driver#1746

Merged

eramongodb approved these changes Oct 2, 2024

View reviewed changes

EC last feedback

aae7635

norareidy merged commit 14e0d91 into mongodb:standardization Oct 2, 2024
5 checks passed

norareidy deleted the DOCSP-43082-cluster-monitoring branch October 2, 2024 20:30

	- Creates a ``mongoc_apm_callbacks_t`` to store callbacks
	- Creates a ``mongoc_apm_callbacks_t`` instance to store callbacks

	- Defines a ``server_opening()`` event handler function, or callback
	- Defines a ``server_opening()`` APM callback function

	- Creates a ``mongoc_apm_callbacks_t`` structure to store callbacks
	- Creates a ``mongoc_apm_callbacks_t`` object to store callbacks

	the callback with the client
	the list of callbacks in the ``mongoc_apm_callbacks_t`` object with the client

	mongoc_client_t *client;
	mongoc_apm_callbacks_t *cbs
	stats_t stats = {0};

	event callback function. The following table provides the name of
	APM callback function. The following table provides the name of

DOCSP-43082: Cluster monitoring #63

DOCSP-43082: Cluster monitoring #63

Uh oh!

Conversation

norareidy commented Oct 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pull Request Info

Self-Review Checklist

Uh oh!

netlify bot commented Oct 1, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

✅ Deploy Preview for docs-c ready!

Uh oh!

mcmorisi left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

eramongodb left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

norareidy commented Oct 1, 2024 •

edited

Loading

netlify bot commented Oct 1, 2024 •

edited

Loading