Skip to content

Fix Sink Metrics Request Latency Metric units#6508

Closed
kkondaka wants to merge 1 commit intoopensearch-project:mainfrom
kkondaka:fix-sink-latency-units
Closed

Fix Sink Metrics Request Latency Metric units#6508
kkondaka wants to merge 1 commit intoopensearch-project:mainfrom
kkondaka:fix-sink-latency-units

Conversation

@kkondaka
Copy link
Copy Markdown
Collaborator

Description

Make Sink Metrics Request Latency Metric units as milliseconds because all other latency metrics in the pipeline are in milliseconds

Issues Resolved

Resolves #[Issue number to be closed when this PR is merged]

Check List

  • New functionality includes testing.
  • New functionality has a documentation issue. Please link to it in this PR.
    • New functionality has javadoc added
  • [X ] Commits are signed with a real name per the DCO

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.
For more information on following Developer Certificate of Origin and signing off your commits, please check here.

Signed-off-by: Kondaka <krishkdk@amazon.com>
SinkFlushResult flushResult = flushableBuffer.flush();
if (flushResult == null) { // success
sinkMetrics.recordRequestLatency((double)(System.nanoTime() - startTime));
double latency = (System.nanoTime() - startTime)/1000_000; // convert to ms
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

This would be clearer: 1_000_000

@dlvenable dlvenable added the breaking change Any change which may break existing configurations and deployments label Feb 13, 2026
@dlvenable dlvenable added this to the v2.14 milestone Feb 13, 2026
@dlvenable
Copy link
Copy Markdown
Member

@kkondaka , I labeled this as a breaking change so that we can note it in the release notes. I think it is the right solution, but we just need to be clear on it.

Copy link
Copy Markdown
Member

@dlvenable dlvenable left a comment

Choose a reason for hiding this comment

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

I have an alternative approach and will send a PR shortly.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking change Any change which may break existing configurations and deployments

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants