Skip to content
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

Support Clearing Custom Dimensions on MetricsLogger #35

Closed
aolundin opened this issue Aug 29, 2020 · 4 comments · Fixed by #105
Closed

Support Clearing Custom Dimensions on MetricsLogger #35

aolundin opened this issue Aug 29, 2020 · 4 comments · Fixed by #105
Assignees
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@aolundin
Copy link

As per README, MetricsLogger.flush() should Flushes the current MetricsContext to the configured sink and resets all properties, dimensions and metric values. The namespace and default dimensions will be preserved across flushes.

In fact, the dimensions and properties on the metricsLogger are retained after flushing.

This unit test illustrates the behavior:

@Test
public void testFlushShouldResetDimensions() {
    logger.putDimensions(DimensionSet.of("foo", "bar"));
    logger.putProperty("property", "propertyValue");

    logger.flush();        

    Assert.assertEquals(sink.getContext().getDimensions().get(0).getDimensionValue("foo"), "bar");
    Assert.assertEquals(1, sink.getContext().getDimensions().size());
    Assert.assertEquals(4, sink.getContext().getDimensions().get(0).getDimensionKeys().size()); // 3 default + 1 custom
    Assert.assertEquals("propertyValue", sink.getContext().getProperty("property"));

    logger.flush();

    Assert.assertEquals(1, sink.getContext().getDimensions().size());
    Assert.assertEquals(3, sink.getContext().getDimensions().get(0).getDimensionKeys().size()); // 3 default + 0 custom // fails here because it's still 4
    Assert.assertNull(sink.getContext().getProperty("property")); // fails here because property is still set
}

Spoke with yaozhaoy about this today, and he said this is actually the desired behavior since metrics Dimension should be preserved across calls to flush for customer convenience. He said he'll update the README accordingly.

My use case is the following: I have a single lambda fronted by several invocation paths from an API gateway. I want a dimension that indicates the invoking path, since the path changes my lambda behavior. There's currently no way to do this with a single metricsLogger since if I were to reset the dimensions, I lose the defaultDimensions (see

)

Since I'm overwriting the dimension with the same key, my CloudWatch metrics actually do come out fine. However, CloudWatch Insights get clobbered with duplicates of each Dimension. Excerpt (ServiceType being my custom dimension):

_aws.CloudWatchMetrics.0.Dimensions.0.0	
LogGroup
_aws.CloudWatchMetrics.0.Dimensions.0.1	
ServiceName
_aws.CloudWatchMetrics.0.Dimensions.0.2	
ServiceType
_aws.CloudWatchMetrics.0.Dimensions.0.3	
ServiceActivity
_aws.CloudWatchMetrics.0.Dimensions.1.0	
LogGroup
_aws.CloudWatchMetrics.0.Dimensions.1.1	
ServiceName
_aws.CloudWatchMetrics.0.Dimensions.1.2	
ServiceType
_aws.CloudWatchMetrics.0.Dimensions.1.3	
ServiceActivity
_aws.CloudWatchMetrics.0.Dimensions.2.0	
LogGroup
_aws.CloudWatchMetrics.0.Dimensions.2.1	
ServiceName
_aws.CloudWatchMetrics.0.Dimensions.2.2	
ServiceType
_aws.CloudWatchMetrics.0.Dimensions.2.3	

The way I'm currently solving this is to force instantiation of a new MetricsLogger for each request to my lambda.

Feature Request: Be able to reset the MetricsLogger for each new request so that I can support different dimensions (and properties etc) from the same logger instance.

@yaozhaoy yaozhaoy self-assigned this Sep 2, 2020
@kasikaruppiah
Copy link

I'm facing a similar issue, the dimensions are retained but the metrics are reset.

@soumyaumass
Copy link

We are facing a similar issue like this. We want to reset statically created MetricsLogger for each Lambda request. We don't want to force instantiate MetricsLogger for each request. Please provide any ETA on this.

@wwaterfield
Copy link

Hi there,

We recently ran into this issue as well. We have use-case where we are setting dimensions & emitting metrics based on the contents of a message we receive, which can vary.

We've worked around this by instantiating a new logger instance when we need to change the dimensions, but this isn't ideal. Is there a plan to fix this behavior?

@aaronlna
Copy link
Contributor

aaronlna commented Sep 14, 2021

hasDefaultDimensions method for MetricsContext is written to always be returning true. This causes flush to effectively skip resetting the MetricsContext and thus dimensions are never actually reset to their defaults.

This looks like an easy fix assuming the README/docs are correct and this is truly a bug and not by design. If this is intentional there needs to be another way to clear dimensions as there's no possible way of doing so without having the consumer control for dimensions manually. Also the docs should get updated if this is the case.

@arielapostoli arielapostoli added bug Something isn't working documentation Improvements or additions to documentation wontfix This will not be worked on labels Jan 7, 2022
@markkuhn markkuhn removed the wontfix This will not be worked on label Jul 20, 2022
@markkuhn markkuhn added enhancement New feature or request and removed bug Something isn't working labels Jul 27, 2022
@markkuhn markkuhn linked a pull request Aug 11, 2022 that will close this issue
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
9 participants