Skip to content
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

chore: moved recorders to lib/metrics/recorders #2666

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

svetlanabrennan
Copy link
Contributor

@svetlanabrennan svetlanabrennan commented Oct 16, 2024

Description

Moved 3 metric recorder functions to lib/metrics/recorders.

How to Test

Run unit tests

Related Issues

Fixes #2656

Copy link

codecov bot commented Oct 16, 2024

Codecov Report

Attention: Patch coverage is 98.76543% with 2 lines in your changes missing coverage. Please review.

Project coverage is 97.18%. Comparing base (b917b3e) to head (5d9e88d).

Files with missing lines Patch % Lines
lib/metrics/recorders/database-operation.js 96.77% 2 Missing ⚠️
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #2666      +/-   ##
==========================================
- Coverage   97.21%   97.18%   -0.03%     
==========================================
  Files         291      294       +3     
  Lines       45928    45968      +40     
==========================================
+ Hits        44650    44676      +26     
- Misses       1278     1292      +14     
Flag Coverage Δ
integration-tests-cjs-18.x 74.18% <46.29%> (+<0.01%) ⬆️
integration-tests-cjs-20.x 74.19% <46.29%> (+<0.01%) ⬆️
integration-tests-cjs-22.x 74.24% <46.29%> (+0.01%) ⬆️
integration-tests-esm-18.x 49.74% <38.27%> (+0.04%) ⬆️
integration-tests-esm-20.x 49.75% <38.27%> (+0.04%) ⬆️
integration-tests-esm-22.x 49.78% <38.27%> (+0.04%) ⬆️
unit-tests-18.x 88.88% <91.97%> (?)
unit-tests-20.x 88.88% <91.97%> (+0.01%) ⬆️
unit-tests-22.x 88.88% <91.97%> (+<0.01%) ⬆️
versioned-tests-18.x 78.96% <97.53%> (-0.14%) ⬇️
versioned-tests-20.x 78.95% <97.53%> (-0.17%) ⬇️
versioned-tests-22.x 78.95% <97.53%> (-0.17%) ⬇️

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

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

@svetlanabrennan svetlanabrennan changed the title chore: moved recorders to lib/metrics/recorders chore: moved recorders to lib/metrics/recorders Oct 17, 2024
@svetlanabrennan svetlanabrennan marked this pull request as ready for review October 17, 2024 12:14
lib/metrics/recorders/record-metrics.js Outdated Show resolved Hide resolved
* @param {string} metricName name of metric
* @returns {Function} recorder for middleware
*/
function makeMiddlewareRecorder(_shim, metricName) {
Copy link
Member

Choose a reason for hiding this comment

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

there's redundancy in the file. i would just name this middleware.js

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I file name or the function name as well?

* @see DatastoreShim#recordOperation
* @see MetricFunction
*/
function recordOperationMetrics(segment, scope) {
Copy link
Member

Choose a reason for hiding this comment

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

i need to study this but seems like this is very similar to the metrics recorder from parsed statement.


'use strict'

const metrics = require('../../metrics/names')
Copy link
Member

Choose a reason for hiding this comment

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

i would rename this to database-operation.js

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Status: Needs PR Review
Development

Successfully merging this pull request may close these issues.

Move middleware, db operation and parsed-statement metric functions to lib/metrics/recorders
2 participants