Skip to content

Conversation

@suresh-prakash
Copy link
Contributor

No description provided.

@suresh-prakash suresh-prakash requested a review from a team as a code owner August 9, 2023 12:28
@github-actions
Copy link

github-actions bot commented Aug 9, 2023

Test Results

31 tests  ±0   31 ✔️ ±0   8s ⏱️ ±0s
  9 suites ±0     0 💤 ±0 
  9 files   ±0     0 ±0 

Results for commit e551b95. ± Comparison against base commit 9a48be3.

♻️ This comment has been updated with latest results.

}

private static void report(final DocStoreMetric metric) {
PlatformMetricsRegistry.registerGauge(metric.name(), metric.labels(), metric.value());
Copy link
Contributor

Choose a reason for hiding this comment

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

we have to call register every time we report?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

We do not need for the standard metrics. I've updated the PR.
But, it may be required for the custom metrics (as the tags might change, rarely).

Copy link
Contributor

Choose a reason for hiding this comment

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

So basically just taking the assumption that there's no harm in calling register repeatedly (may very well be true, no idea)

Copy link
Contributor Author

@suresh-prakash suresh-prakash Aug 12, 2023

Choose a reason for hiding this comment

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

Functionally, I tested it and it works. Performance-wise, I'm not sure. But, I don't think performance is critical here (as long as there isn't a huge variation in performance, which may not be the case).

@suresh-prakash suresh-prakash changed the title Added a monitor method for database monitoring through document store Added 'monitor' and 'report' methods for database monitoring through document store Aug 11, 2023
@codecov
Copy link

codecov bot commented Aug 11, 2023

Codecov Report

Merging #75 (e551b95) into main (9a48be3) will not change coverage.
The diff coverage is n/a.

@@            Coverage Diff            @@
##               main      #75   +/-   ##
=========================================
  Coverage     70.21%   70.21%           
  Complexity      106      106           
=========================================
  Files            15       15           
  Lines           591      591           
  Branches         32       32           
=========================================
  Hits            415      415           
  Misses          157      157           
  Partials         19       19           
Flag Coverage Δ
unit 70.21% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

}

private static void report(final DocStoreMetric metric) {
PlatformMetricsRegistry.registerGauge(metric.name(), metric.labels(), metric.value());
Copy link
Contributor

Choose a reason for hiding this comment

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

So basically just taking the assumption that there's no harm in calling register repeatedly (may very well be true, no idea)

@suresh-prakash suresh-prakash merged commit 7ee8a12 into main Aug 15, 2023
@suresh-prakash suresh-prakash deleted the add_database_monitor_registry branch August 15, 2023 04:53
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.

3 participants