Skip to content

feat: Metrics utility #91

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 21 commits into from
Sep 22, 2020
Merged

feat: Metrics utility #91

merged 21 commits into from
Sep 22, 2020

Conversation

pankajagrawal16
Copy link
Contributor

@pankajagrawal16 pankajagrawal16 commented Sep 12, 2020

Issue #, if available:

Description of changes:

EMF logging support

Checklist

Breaking change checklist

RFC issue #:

  • Migration process documented
  • Implement warnings (if it can live side by side)

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

@pankajagrawal16 pankajagrawal16 force-pushed the metrics branch 4 times, most recently from 4307dd9 to 3b7c97c Compare September 12, 2020 08:06
@pankajagrawal16 pankajagrawal16 changed the title Metrics feat: Metrics Sep 12, 2020
@pankajagrawal16 pankajagrawal16 force-pushed the metrics branch 2 times, most recently from 79e9306 to 1f46b59 Compare September 12, 2020 08:37
@pankajagrawal16 pankajagrawal16 changed the title feat: Metrics feat: Metrics utility Sep 12, 2020
@msailes
Copy link
Contributor

msailes commented Sep 12, 2020

I'll write some Javadocs.

}

private String namespace(PowertoolsMetrics powertoolsMetrics) {
return !"".equals(powertoolsMetrics.namespace()) ? powertoolsMetrics.namespace() : NAMESPACE;
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the default namespace if neither the annotation parameter or environment variable are set?

Copy link
Contributor

Choose a reason for hiding this comment

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

This should probably match the "service_undefined" functionality.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

return metricsLogger;
}

public static void withSingleMetric(final String name,
Copy link
Contributor

Choose a reason for hiding this comment

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

Did you leave the service out on purpose?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@pankajagrawal16 pankajagrawal16 force-pushed the metrics branch 11 times, most recently from c3120b5 to e0db6bc Compare September 13, 2020 16:52
@pankajagrawal16
Copy link
Contributor Author

For some reason which I am not able to figure out yet, java doc plugin fails with error as below on java9 and 10.

[ERROR] /home/runner/work/aws-lambda-powertools-java/aws-lambda-powertools-java/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/PowertoolsMetricsLogger.java:12: warning - invalid usage of tag {@see PowertoolsMetrics}
[ERROR] javadoc: error - An exception occurred while building a component: PackageSerializedForm
[ERROR] 	(com.sun.tools.javac.code.Symbol$CompletionFailure: class file for com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter not found)
[ERROR] Please file a bug against the javadoc tool via the Java bug reporting page
[ERROR] (http://bugreport.java.com) after checking the Bug Database (http://bugs.java.com)
[ERROR] for duplicates. Include error messages and the following diagnostic in your report. Thank you.
[ERROR] com.sun.tools.javac.code.Symbol$CompletionFailure: class file for com.fasterxml.jackson.databind.ser.impl.SimpleBeanPropertyFilter not found
[ERROR] 
[ERROR] Command line was: /opt/hostedtoolcache/jdk/9.0.7/x64/bin/javadoc @options @packages
[ERROR] 
[ERROR] Refer to the generated Javadoc files in '/home/runner/work/aws-lambda-powertools-java/aws-lambda-powertools-java/powertools-metrics/target/apidocs' dir.
[ERROR] -> [Help 1]
[ERROR] 

@pankajagrawal16
Copy link
Contributor Author

pankajagrawal16 commented Sep 13, 2020

This commit enables validations as well like mentioned here https://awslabs.github.io/aws-lambda-powertools-python/core/metrics/#flushing-metrics. Which means its full feature parity now with python version.

@KassHino
Copy link

Quick question: How often does these get reviewed? And how long can these reviews take? I ask because I'm wondering if I should integrate with the EMF java library, or wait for this PR to get reviewed and merged?

@pankajagrawal16
Copy link
Contributor Author

Quick question: How often does these get reviewed? And how long can these reviews take? I ask because I'm wondering if I should integrate with the EMF java library, or wait for this PR to get reviewed and merged?

Hi Kassandra,

This should be released soon.

Copy link
Contributor

@msailes msailes left a comment

Choose a reason for hiding this comment

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

Approved

@msailes msailes merged commit a22bf2d into master Sep 22, 2020
@msailes msailes deleted the metrics branch September 22, 2020 09:02
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