MetricsUtils.withSingleMetric doesn't publish powertools default "Service" dimension

[qtdzz]

What were you trying to accomplish?
I want to use MetricsUtils.withSingleMetric to publish a single metric with additional dimensions without affecting dimensions of other metrics. The single metric should also have the same default powertools dimension as other metrics when I don't overriding the default dimensions.

Expected Behavior

The single metric should have the default "Service" dimension as other normal metrics

Current Behavior

The single metric doesn't have the default "Service" dimension as other normal metrics

Possible Solution

I think the problem is in MetricsUtils#logger. I suspect two issues here:

1. If there is no overriding default dimensions, we skip setting default dimensions. But in LambdaMetricsAspect#refreshMetricsContext (normal cases) we are appending the default powertools Service dimension instead. Possible solution: appending default Powertools Service dimension here as well.

2. By using setDimensions if there are default overriding dimensions, this will likely cause another issue when customers use metric.setDimensions(dimensions); in their Consumer<MetricsLogger>, the overriding default dimensions will be gone. Possible solution: use setDefaultDimensions and update documentation to advice customers using putDimensions instead of setDimensions (because using setDimensions will hide all the default dimensions)

Steps to Reproduce (for bugs)

1. Don't overwrite defaultDimensions.
2. Put some metrics using MetricsUtils.metricsLogger() so my metrics will have the default "Service" dimension created by Lambda Powertools
3. Publish a single metric with an additional dimension as following, the metric will only have AdditionalDimension

        MetricsUtils.withSingleMetric("MyCoolMetric", 1, Unit.COUNT, metric -> {
                final DimensionSet dimensions = DimensionSet.of("AdditionalDimension", "AdditionalDimensionValue");
                metric.setDimensions(dimensions);
            });

4. If Using metric.putDimension(dimensions) instead of setDimensions, you will get all default EMF dimensions such as LogGroup, ServiceName, ServiceType which is also not desirable, I think.

Environment

• Powertools version used: v1.5.0
• Packaging format (Layers, Maven/Gradle): Maven
• AWS Lambda function runtime: Java8
• Debugging logs: N/A

[pankajagrawal16]

Hey @qtdzz Thanks for opening the issue, I will have a look at this later today.

[pankajagrawal16]

@qtdzz I have read through the details of this issue now. Some thoughts.

• This was intentionally not supported. So its not a specifically a bug.
• While I do believe it will be a good user experience, but there are some challenges implementing it. As of today with @Tracing annotation and its behaviour of setting default dimension as Service, below is the order of precedence:

1. Check for default overriding via MetricsUtils.getDefaultDimensions() and set default dimension as is across all api calls.
2. Else, Check if service attribute specified on @Tracing annotation, if found use it as Service dimension.
3. Else Check if POWERTOOLS_SERVICE_NAME environment var set, if found use it as Service dimension.

Challenge here is to keep this precedence consistent MetricsUtils.withSingleMetric api as well since we don't have access to @Tracing annotation service attribute.

That being said, since this is related to behavior in core utilities of powertools, I quickly checked the behavior for python version as well. It does not support this feature as well.

@heitorlessa What are your thoughts on this?

[qtdzz]

Thank you @pankajagrawal16 for looking into the issue and the information. Now I get the challenging part.

2. Else, Check if service attribute specified on @Tracing annotation, if found use it as Service dimension

I assume you are talking about the @Metric annotation.

As far as I understand, the steps you described to resolve default powertools dimension are all in LambdaMetricsAspect.refreshMetricsContext. Could we keep the value after resolving as a static variable in the LambdaMetricsAspect class or any other within the internal package? So that customers will know that they shouldn't use it. Then we can take it as a fallback in MetricsUtils.withSingleMetric when there is no default dimension.

It's indeed not a clean solution but from consumers perspective, I think it provides much better experience.

[pankajagrawal16]

Thank you @pankajagrawal16 for looking into the issue and the information. Now I get the challenging part.

1. Else, Check if service attribute specified on @Tracing annotation, if found use it as Service dimension

I assume you are talking about the @Metric annotation.

As far as I understand, the steps you described to resolve default powertools dimension are all in LambdaMetricsAspect.refreshMetricsContext. Could we keep the value after resolving as a static variable in the LambdaMetricsAspect class or any other within the internal package? So that customers will know that they shouldn't use it. Then we can take it as a fallback in MetricsUtils.withSingleMetric when there is no default dimension.

It's indeed not a clean solution but from consumers perspective, I think it provides much better experience.

Apologies, Yeah I meant @Metrics annotation.

Implementation is not an issue, we might support it. At this point I am wondering if it's too much assumptions to make for the users. And since this is a core utility, i will be much interested in learning what python folks think about this support.

@heitorlessa May be you missed this one, thoughts?

[heitorlessa]

We did or nay internationally - single metric is for customers looking to create a single metric that would differ from dimensions and other metrics already set. I’m totally onboard to have an additional parameter within single metric to include service dimension and/or default dimensions too —- this however begs the question as to why you’d want both service and default dimensions in this case vs Metrics. If there’s a proposed UX, let’s create a roadmap item to discuss more narrowly ;-)

…

On Wed, 2 Jun 2021 at 10:06, Pankaj Agrawal ***@***.***> wrote: Thank you @pankajagrawal16 <https://github.com/pankajagrawal16> for looking into the issue and the information. Now I get the challenging part. 1. Else, Check if service attribute specified on @Tracing annotation, if found use it as Service dimension I assume you are talking about the @Metric annotation. As far as I understand, the steps you described to resolve default powertools dimension are all in LambdaMetricsAspect.refreshMetricsContext <https://github.com/awslabs/aws-lambda-powertools-java/blob/master/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspect.java#L122>. Could we keep the value after resolving as a static variable in the LambdaMetricsAspect class or any other within the internal package? So that customers will know that they shouldn't use it. Then we can take it as a fallback in MetricsUtils.withSingleMetric when there is no default dimension. It's indeed not a clean solution but from consumers perspective, I think it provides much better experience. Apologies, Yeah I meant @metrics annotaion. Implementation is not an issue, we might support it. At this point I am wondering if it's too much assumptions to make for the users. And since this is a core utility, i will be much interested in learning what python folks think about this support. @heitorlessa <https://github.com/heitorlessa> May be you missed this one, thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/awslabs/aws-lambda-powertools-java/issues/409#issuecomment-852832770>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZPQBD3JXU65WC4NZIQC5TTQXRCDANCNFSM45ZH3Y3A> .

[pankajagrawal16]

We did or nay internationally - single metric is for customers looking to create a single metric that would differ from dimensions and other metrics already set. I’m totally onboard to have an additional parameter within single metric to include service dimension and/or default dimensions too —- this however begs the question as to why you’d want both service and default dimensions in this case vs Metrics. If there’s a proposed UX, let’s create a roadmap item to discuss more narrowly ;-)
…
On Wed, 2 Jun 2021 at 10:06, Pankaj Agrawal @.***> wrote: Thank you @pankajagrawal16 https://github.com/pankajagrawal16 for looking into the issue and the information. Now I get the challenging part. 1. Else, Check if service attribute specified on @Tracing annotation, if found use it as Service dimension I assume you are talking about the @Metric annotation. As far as I understand, the steps you described to resolve default powertools dimension are all in LambdaMetricsAspect.refreshMetricsContext https://github.com/awslabs/aws-lambda-powertools-java/blob/master/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspect.java#L122. Could we keep the value after resolving as a static variable in the LambdaMetricsAspect class or any other within the internal package? So that customers will know that they shouldn't use it. Then we can take it as a fallback in MetricsUtils.withSingleMetric when there is no default dimension. It's indeed not a clean solution but from consumers perspective, I think it provides much better experience. Apologies, Yeah I meant @metrics annotaion. Implementation is not an issue, we might support it. At this point I am wondering if it's too much assumptions to make for the users. And since this is a core utility, i will be much interested in learning what python folks think about this support. @heitorlessa https://github.com/heitorlessa May be you missed this one, thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#409 (comment)>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZPQBD3JXU65WC4NZIQC5TTQXRCDANCNFSM45ZH3Y3A .

Makes sense, There are no changes to UX. The withSingleMetric APIs remains the same just that it will make some assumptions and automatically capture Service as dimension based on what's defined in environment var/at annotation level/default dimension.

But it makes sense, that since this probably changes to behavior of core utility, I will create a roadmap item and start discussion there

[heitorlessa]

I’d suggest not doing that but making an opt-in behaviour. Adding dimension based on these assumptions not only creates side effects but it’ll also lead to higher costs on their bill - Adding a dimension effectively creates another Metric as it’s a tuple (metric+dimension) On Wed, 2 Jun 2021 at 13:13, Pankaj Agrawal ***@***.***> wrote:

…

We did or nay internationally - single metric is for customers looking to create a single metric that would differ from dimensions and other metrics already set. I’m totally onboard to have an additional parameter within single metric to include service dimension and/or default dimensions too —- this however begs the question as to why you’d want both service and default dimensions in this case vs Metrics. If there’s a proposed UX, let’s create a roadmap item to discuss more narrowly ;-) … <#m_982134101925854959_> On Wed, 2 Jun 2021 at 10:06, Pankaj Agrawal *@*.***> wrote: Thank you @pankajagrawal16 <https://github.com/pankajagrawal16> https://github.com/pankajagrawal16 for looking into the issue and the information. Now I get the challenging part. 1. Else, Check if service attribute specified on @Tracing <https://github.com/Tracing> annotation, if found use it as Service dimension I assume you are talking about the @Metric <https://github.com/Metric> annotation. As far as I understand, the steps you described to resolve default powertools dimension are all in LambdaMetricsAspect.refreshMetricsContext https://github.com/awslabs/aws-lambda-powertools-java/blob/master/powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspect.java#L122. Could we keep the value after resolving as a static variable in the LambdaMetricsAspect class or any other within the internal package? So that customers will know that they shouldn't use it. Then we can take it as a fallback in MetricsUtils.withSingleMetric when there is no default dimension. It's indeed not a clean solution but from consumers perspective, I think it provides much better experience. Apologies, Yeah I meant @metrics <https://github.com/metrics> annotaion. Implementation is not an issue, we might support it. At this point I am wondering if it's too much assumptions to make for the users. And since this is a core utility, i will be much interested in learning what python folks think about this support. @heitorlessa <https://github.com/heitorlessa> https://github.com/heitorlessa May be you missed this one, thoughts? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#409 (comment) <https://github.com/awslabs/aws-lambda-powertools-java/issues/409#issuecomment-852832770>>, or unsubscribe https://github.com/notifications/unsubscribe-auth/AAZPQBD3JXU65WC4NZIQC5TTQXRCDANCNFSM45ZH3Y3A . Makes sense, There are no changes to UX. The withSingleMetric APIs remains the same just that it will make some assumptions and automatically capture Service as dimension based on what's defined in environment var/at annotation level/default dimension. But it makes sense, that since this probably changes to behavior of core utility, I will create a roadmap item and start discussion there — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <https://github.com/awslabs/aws-lambda-powertools-java/issues/409#issuecomment-852939458>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/AAZPQBH45S4IINTUUCGE54DTQYG5ZANCNFSM45ZH3Y3A> .

[pankajagrawal16]

I’d suggest not doing that but making an opt-in behaviour. Adding dimension based on these assumptions not only creates side effects but it’ll also lead to higher costs on their bill - Adding a dimension effectively creates another Metric as it’s a tuple (metric+dimension) On Wed, 2 Jun 2021 at 13:13, Pankaj Agrawal @.***> wrote:

Yeah makes sense, Opt in is better.

[pankajagrawal16]

Since this pertains to core utility Metrics, Moving here for better visibility and discussion.