Skip to content

feat(metrics): New metrics module implementation with support for Metrics providers and usage without annotations #1863

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

Merged
merged 36 commits into from
Jun 6, 2025

Conversation

phipag
Copy link
Contributor

@phipag phipag commented Jun 4, 2025

Issue #, if available: #1848

Description of changes:

This PR re-implements the metrics module providing an interface fully owned by Powertools. It lays the foundation to support multiple Metrics providers in the future (see related TypeScript issue aws-powertools/powertools-lambda-typescript#1812).

This PR:

  • Rewrites the metrics module decoupling the interface completely from the EMF library
    • In the future, we can remove the EMF dependency (will be separate work)
  • Provides a fully working functional interface that allows using the utility without @Metrics annotation
    • Similar to .NET it supports the Builder pattern now using MetricsLoggerBuilder
  • Updates documentation with new module and some additional hints and descriptions
  • Adds more extensive unit tests covering 100% statements (only some partial if-branches are missed)
  • Updates GraalVM metadata files with new interface and new unit tests using an updated tracing agent
    • I tested that this works in my AWS account

Documentation preview deployed here: https://dealn7fl31ram.cloudfront.net/core/metrics/

Checklist

Breaking change checklist

  • Note: I will update the upgrade guide in the documentation in a separate PR. I would like to get a final version of this with feedback first.

By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice.

…csLogger to enable user configuration of the MetricsLogger singleton.
@dreamorosi dreamorosi self-requested a review June 4, 2025 14:27
Copy link
Contributor

@dreamorosi dreamorosi left a comment

Choose a reason for hiding this comment

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

Thanks for the PR!

As discussed in the review session, let's address a few minor details around:

  • supporting custom timestamps + validation
  • removing service name dimension when not set
  • aligning behavior on overwriting dimensions & metadata (wherever possible)
  • considering changing name of single metric method to use the verb flush
  • adding a section about testing your code (nice to have)

@phipag
Copy link
Contributor Author

phipag commented Jun 5, 2025

Hey @dreamorosi, thanks again for the in-depth review. I implemented your changes plus some additional things I found.

  • If no Service is defined, we don't create the Service default dimension anymore
  • Fail if not namespace is set instead of using EMF library default one
  • Add custom Validators for Timestamp, namespace, and dimensions
  • Update documentation with various changes and additional examples
  • Add support for POWERTOOLS_METRICS_DISABLED environment variable
  • Add example for adding high cardinality dimensions in documentation
  • Update Maximum Dimensions to 30 according to EMF spec
  • Accept a DimensionSet for addDimension and setDefaultDimensions methods instead of a generic Java Map
  • Add support for FUNCTION_NAME environment variable
  • Log a warning if metrics are empty and raiseOnEmptyMetrics is false
  • Add support for specifying Timestamps using setTimestamp
  • Rename @Metrics to @FlushMetrics and rename MetricsLogger to Metrics along with Factory and Builder
  • Rename pushSingleMetric to flushSingleMetric
  • Add Testing section into documentation
  • Fix e2e tests

@phipag
Copy link
Contributor Author

phipag commented Jun 5, 2025

E2E tests pass without any changes in assertions: https://github.com/aws-powertools/powertools-lambda-java/actions/runs/15469183318

@dreamorosi
Copy link
Contributor

I see we accepted a SonarQube finding around complexity, is there any way at all that we can break the logic down in smaller chunks? If not, then I'm ok to keep it.

@phipag
Copy link
Contributor Author

phipag commented Jun 6, 2025

I see we accepted a SonarQube finding around complexity, is there any way at all that we can break the logic down in smaller chunks? If not, then I'm ok to keep it.

3d02853

Sonar passes now.

Copy link

sonarqubecloud bot commented Jun 6, 2025

@phipag
Copy link
Contributor Author

phipag commented Jun 6, 2025

I also applied some changes to the GraalVM metadata files after renaming the MetricsLogger. 53d054d

@phipag
Copy link
Contributor Author

phipag commented Jun 6, 2025

Hey @dreamorosi, thanks again for your feedback. This PR is ready for review again.

@dreamorosi dreamorosi self-requested a review June 6, 2025 07:56
@phipag phipag merged commit 57ae153 into v2 Jun 6, 2025
18 checks passed
@phipag phipag deleted the phipag/issue1848 branch June 6, 2025 09:56
@github-project-automation github-project-automation bot moved this from Pending review to Coming soon in Powertools for AWS Lambda (Java) Jun 6, 2025
@phipag phipag linked an issue Jun 6, 2025 that may be closed by this pull request
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature-parity Feature parity with python version maintenance metrics size/XXL
Projects
Status: Coming soon
Development

Successfully merging this pull request may close these issues.

Tech debt: Create abstraction for Metrics and do not expose aws-embedded-metrics-java Feature request: Support setting functionName for cold-starts
2 participants