Skip to content

Add sanitization to metric names and labels#6277

Merged
kkondaka merged 5 commits into
opensearch-project:mainfrom
kkondaka:amp-sink-fixes
Nov 19, 2025
Merged

Add sanitization to metric names and labels#6277
kkondaka merged 5 commits into
opensearch-project:mainfrom
kkondaka:amp-sink-fixes

Conversation

@kkondaka

Copy link
Copy Markdown
Collaborator

Description

Add sanitization to metric names and labels

Issues Resolved

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

Check List

  • [ X] 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>
awsCredentialsSupplier.getProvider(convertToCredentialOptions(config.getAwsConfig())) :
awsCredentialsSupplier.getProvider(AwsCredentialsOptions.builder().build());

/*

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.

Please delete this code entirely.

import org.junit.jupiter.api.Test;
import org.mockito.Mock;
import static org.mockito.ArgumentMatchers.any;

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.

Let's add the auto-test annotation to this test or to a new test. It automatically generates a couple of tests to ensure the plugin is correct. With more tests to come.


private static String sanitizeMetricName(String name) {
return sanitizeName(name, true); // metric names allow colon
// See https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/pkg/translator/prometheus

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.

Turn this into a Javadoc comment.

/**
 * @see https://github.com/open-telemetry/opentelemetry-collector-contrib/tree/main/pkg/translator/prometheus
 */

this.config = config;
this.credentialsProvider = awsCredentialsSupplier.getProvider(AwsCredentialsOptions.builder()
this.credentialsProvider = (config.getAwsConfig() != null) ?
awsCredentialsSupplier.getProvider(convertToCredentialOptions(config.getAwsConfig())) :

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.

Nit: Would be a little cleaner to just have the method convertToCredentialOptions do the logic to get if aws config is null

graytaylor0
graytaylor0 previously approved these changes Nov 18, 2025
Signed-off-by: Kondaka <krishkdk@amazon.com>
Signed-off-by: Kondaka <krishkdk@amazon.com>
dlvenable
dlvenable previously approved these changes Nov 19, 2025
Signed-off-by: Kondaka <krishkdk@amazon.com>
Signed-off-by: Kondaka <krishkdk@amazon.com>
@kkondaka kkondaka merged commit ae5df65 into opensearch-project:main Nov 19, 2025
48 of 51 checks passed
eatulban pushed a commit to eatulban/data-prepper that referenced this pull request Dec 11, 2025
* Add sanitization to metric names and labels

Signed-off-by: Kondaka <krishkdk@amazon.com>

* Addressed review  comments

Signed-off-by: Kondaka <krishkdk@amazon.com>

* Fixed build issues

Signed-off-by: Kondaka <krishkdk@amazon.com>

* Addressed comments

Signed-off-by: Kondaka <krishkdk@amazon.com>

* Removed unnecessary debug statement

Signed-off-by: Kondaka <krishkdk@amazon.com>

---------

Signed-off-by: Kondaka <krishkdk@amazon.com>
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