Skip to content

Mertics default dimensions #329

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.

Sign up for GitHub

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 6 commits into from
Mar 23, 2021
Merged

Mertics default dimensions #329

Show file tree
Hide file tree
Changes from 4 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
6164c3f
feat: support for setting list of default dimensions
Mar 15, 2021
9f76dc8
docs: support for setting list of default dimensions
Mar 15, 2021
7899590
chore: fix docs build
Mar 15, 2021
3670461
chore: fix failing failing build
Mar 15, 2021
3144d03
chore: remove redudant use of setting defautl dimension
Mar 15, 2021
02e43bf
chore: clarify UX for setting default dimension with no dimension via…
Mar 15, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 3 additions & 1 deletion .github/workflows/build-docs.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,4 +27,6 @@ jobs:
echo "SOURCE_BRANCH=${GITHUB_REF#refs/heads/}" >> $GITHUB_ENV
echo "SOURCE_TAG=${GITHUB_REF#refs/tags/}" >> $GITHUB_ENV
- name: Build docs website
run: make build-docs-website
run: |
echo "GIT_PYTHON_REFRESH=quiet"
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 this for?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

There was a failing build on docs and some research suggested to use it coz of some git behaviour changes

make build-docs-website
4 changes: 2 additions & 2 deletions docs/core/metrics.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -186,7 +186,7 @@ This will be available in CloudWatch Logs to ease operations on high cardinal da

By default, all metrics emitted via module captures `Service` as one of the default dimension. This is either specified via
`POWERTOOLS_SERVICE_NAME` environment variable or via `service` attribute on `Metrics` annotation. If you wish to override the default
Dimension, it can be done via `#!java MetricsUtils.defaultDimensionSet()`.
Dimension, it can be done via `#!java MetricsUtils.defaultDimensions()`.

=== "App.java"

Expand All @@ -199,7 +199,7 @@ Dimension, it can be done via `#!java MetricsUtils.defaultDimensionSet()`.
MetricsLogger metricsLogger = MetricsUtils.metricsLogger();
static {
MetricsUtils.defaultDimensionSet(DimensionSet.of("CustomDimension", "booking"));
MetricsUtils.defaultDimensions(DimensionSet.of("CustomDimension", "booking"));
}

@Override
Expand Down
28 changes: 22 additions & 6 deletions powertools-metrics/src/main/java/software/amazon/lambda/powertools/metrics/MetricsUtils.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@
*/
public final class MetricsUtils {
private static final MetricsLogger metricsLogger = new MetricsLogger();
private static DimensionSet defaultDimensionSet;
private static DimensionSet[] defaultDimensions;

private MetricsUtils() {
}
Expand All @@ -39,14 +39,29 @@ public static MetricsLogger metricsLogger() {
return metricsLogger;
}

/**
* Configure default dimension to be used by logger.
* By default, @{@link Metrics} annotation captures configured service as a dimension <i>Service</i>
* @param dimensionSets Default value of dimensions set for logger
*/
public static void defaultDimensions(final DimensionSet... dimensionSets) {
MetricsUtils.defaultDimensions = dimensionSets;
}

/**
* Configure default dimension to be used by logger.
* By default, @{@link Metrics} annotation captures configured service as a dimension <i>Service</i>
* @param dimensionSet Default value of dimension set for logger
* @deprecated use {@link #defaultDimensions(DimensionSet...)} instead
*
*/
@Deprecated
public static void defaultDimensionSet(final DimensionSet dimensionSet) {
requireNonNull(dimensionSet, "Null dimension set not allowed");
MetricsUtils.defaultDimensionSet = dimensionSet;

if(dimensionSet.getDimensionKeys().size() > 0) {
MetricsUtils.defaultDimensions = new DimensionSet[]{dimensionSet};
Copy link
Contributor

Choose a reason for hiding this comment

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

I think MetricsUtils.defaultDimensions(dimensionSet) should do the trick here

}
}


Expand Down Expand Up @@ -105,12 +120,12 @@ public static void withSingleMetric(final String name,
}
}

public static DimensionSet defaultDimensionSet() {
return defaultDimensionSet;
public static DimensionSet[] defaultDimensions() {
return defaultDimensions;
}

public static boolean hasDefaultDimension() {
return null != defaultDimensionSet && defaultDimensionSet.getDimensionKeys().size() > 0;
return null != defaultDimensions;
}

private static void captureRequestAndTraceId(MetricsLogger metricsLogger) {
Expand All @@ -137,7 +152,8 @@ private static MetricsLogger logger() {
MetricsContext metricsContext = new MetricsContext();

if (hasDefaultDimension()) {
metricsContext.setDefaultDimensions(defaultDimensionSet());
metricsContext.setDefaultDimensions(new DimensionSet());
Copy link
Contributor

Choose a reason for hiding this comment

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

I think this is not needed as setDimensions overrides the defaults or is there something am missing?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

That is true.

metricsContext.setDimensions(defaultDimensions);
}

return new MetricsLogger(new EnvironmentProvider(), metricsContext);
Expand Down
9 changes: 5 additions & 4 deletions ...src/main/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspect.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,7 @@
import software.amazon.cloudwatchlogs.emf.model.Unit;
import software.amazon.lambda.powertools.core.internal.LambdaHandlerProcessor;
import software.amazon.lambda.powertools.metrics.Metrics;
import software.amazon.lambda.powertools.metrics.MetricsUtils;
import software.amazon.lambda.powertools.metrics.ValidationException;

import static software.amazon.cloudwatchlogs.emf.model.MetricsLoggerHelper.dimensionsCount;
Expand All @@ -23,7 +24,6 @@
import static software.amazon.lambda.powertools.core.internal.LambdaHandlerProcessor.placedOnRequestHandler;
import static software.amazon.lambda.powertools.core.internal.LambdaHandlerProcessor.placedOnStreamHandler;
import static software.amazon.lambda.powertools.core.internal.LambdaHandlerProcessor.serviceName;
import static software.amazon.lambda.powertools.metrics.MetricsUtils.defaultDimensionSet;
import static software.amazon.lambda.powertools.metrics.MetricsUtils.hasDefaultDimension;
import static software.amazon.lambda.powertools.metrics.MetricsUtils.metricsLogger;

Expand Down Expand Up @@ -125,10 +125,11 @@ public static void refreshMetricsContext(Metrics metrics) {
f.setAccessible(true);
MetricsContext context = new MetricsContext();

DimensionSet defaultDimensionSet = hasDefaultDimension() ? defaultDimensionSet()
: DimensionSet.of("Service", service(metrics));
DimensionSet[] defaultDimensions = hasDefaultDimension() ? MetricsUtils.defaultDimensions()
: new DimensionSet[]{DimensionSet.of("Service", service(metrics))};

context.setDefaultDimensions(defaultDimensionSet);
context.setDefaultDimensions(new DimensionSet());
context.setDimensions(defaultDimensions);

f.set(metricsLogger(), context);
} catch (NoSuchFieldException | IllegalAccessException e) {
Expand Down
2 changes: 1 addition & 1 deletion ...ls-metrics/src/test/java/software/amazon/lambda/powertools/metrics/MetricsLoggerTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,7 +52,7 @@ void singleMetricsCaptureUtilityWithDefaultDimension() {
mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");
internalWrapper.when(() -> getenv("_X_AMZN_TRACE_ID")).thenReturn("Root=1-5759e988-bd862e3fe1be46a994272793;Parent=53995c3f42cd8ad8;Sampled=1\"");

MetricsUtils.defaultDimensionSet(DimensionSet.of("Service", "Booking"));
MetricsUtils.defaultDimensions(DimensionSet.of("Service", "Booking"));

MetricsUtils.withSingleMetric("Metric1", 1, Unit.COUNT, "test",
metricsLogger -> {});
Expand Down
4 changes: 2 additions & 2 deletions ...n/lambda/powertools/metrics/handlers/PowertoolsMetricsEnabledDefaultDimensionHandler.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,14 +7,14 @@
import software.amazon.cloudwatchlogs.emf.model.Unit;
import software.amazon.lambda.powertools.metrics.Metrics;

import static software.amazon.lambda.powertools.metrics.MetricsUtils.defaultDimensionSet;
import static software.amazon.lambda.powertools.metrics.MetricsUtils.defaultDimensions;
import static software.amazon.lambda.powertools.metrics.MetricsUtils.metricsLogger;
import static software.amazon.lambda.powertools.metrics.MetricsUtils.withSingleMetric;

public class PowertoolsMetricsEnabledDefaultDimensionHandler implements RequestHandler<Object, Object> {

static {
defaultDimensionSet(DimensionSet.of("CustomDimension", "booking"));
defaultDimensions(DimensionSet.of("CustomDimension", "booking"));
}

@Override
Expand Down
21 changes: 10 additions & 11 deletions ...test/java/software/amazon/lambda/powertools/metrics/internal/LambdaMetricsAspectTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,6 @@ public class LambdaMetricsAspectTest {
private final PrintStream originalOut = System.out;
private final ObjectMapper mapper = new ObjectMapper();
private RequestHandler<Object, Object> requestHandler;
private RequestStreamHandler streamHandler;


@BeforeAll
Expand Down Expand Up @@ -80,7 +79,7 @@ public void metricsWithoutColdStart() {
mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");
internalWrapper.when(() -> getenv("_X_AMZN_TRACE_ID")).thenReturn("Root=1-5759e988-bd862e3fe1be46a994272793;Parent=53995c3f42cd8ad8;Sampled=1\"");

MetricsUtils.defaultDimensionSet(new DimensionSet());
MetricsUtils.defaultDimensions(null);
Copy link
Contributor

Choose a reason for hiding this comment

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

I am not quite sure I follow what setting it to null means here? if no dimensions, shouldn't it just be MetricsUtils.defaultDimensions()? or is this because of getter/setter having the same name? If so, then the case for setting no dimensions as a default is a bit complicated UX-wise.

Copy link
Contributor Author

@pankajagrawal16 pankajagrawal16 Mar 15, 2021 •
edited
Loading

Choose a reason for hiding this comment

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

Its more of a limitation of how MockedStatic works and test specific.

Ux for setting no dimension is now added as part of a test as well https://github.com/awslabs/aws-lambda-powertools-java/blob/02e43bf22fbdea163ae5663049d447d3c8c65f52/powertools-metrics/src/test/java/software/amazon/lambda/powertools/metrics/handlers/PowertoolsMetricsEnabledDefaultNoDimensionHandler.java#L16

requestHandler = new PowertoolsMetricsEnabledHandler();
requestHandler.handleRequest("input", context);

Expand Down Expand Up @@ -161,7 +160,7 @@ public void metricsWithColdStart() {

mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");

MetricsUtils.defaultDimensionSet(new DimensionSet());
MetricsUtils.defaultDimensions(null);
requestHandler = new PowertoolsMetricsColdStartEnabledHandler();

requestHandler.handleRequest("input", context);
Expand Down Expand Up @@ -196,7 +195,7 @@ public void noColdStartMetricsWhenColdStartDone() {
try (MockedStatic<SystemWrapper> mocked = mockStatic(SystemWrapper.class)) {
mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");

MetricsUtils.defaultDimensionSet(new DimensionSet());
MetricsUtils.defaultDimensions(null);
requestHandler = new PowertoolsMetricsColdStartEnabledHandler();

requestHandler.handleRequest("input", context);
Expand Down Expand Up @@ -241,8 +240,8 @@ public void metricsWithStreamHandler() throws IOException {
try (MockedStatic<SystemWrapper> mocked = mockStatic(SystemWrapper.class)) {
mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");

MetricsUtils.defaultDimensionSet(new DimensionSet());
streamHandler = new PowertoolsMetricsEnabledStreamHandler();
MetricsUtils.defaultDimensions(null);
RequestStreamHandler streamHandler = new PowertoolsMetricsEnabledStreamHandler();

streamHandler.handleRequest(new ByteArrayInputStream(new byte[]{}), new ByteArrayOutputStream(), context);

Expand All @@ -264,7 +263,7 @@ public void exceptionWhenNoMetricsEmitted() {
try (MockedStatic<SystemWrapper> mocked = mockStatic(SystemWrapper.class)) {
mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");

MetricsUtils.defaultDimensionSet(new DimensionSet());
MetricsUtils.defaultDimensions((DimensionSet) null);
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: I see other statement not having to use the type casting, is it really needed here?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

No, its not . Just a miss

requestHandler = new PowertoolsMetricsExceptionWhenNoMetricsHandler();

assertThatExceptionOfType(ValidationException.class)
Expand All @@ -279,7 +278,7 @@ public void noExceptionWhenNoMetricsEmitted() {
try (MockedStatic<SystemWrapper> mocked = mockStatic(SystemWrapper.class)) {
mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");

MetricsUtils.defaultDimensionSet(new DimensionSet());
MetricsUtils.defaultDimensions(null);
requestHandler = new PowertoolsMetricsNoExceptionWhenNoMetricsHandler();

requestHandler.handleRequest("input", context);
Expand All @@ -299,7 +298,7 @@ public void noExceptionWhenNoMetricsEmitted() {
public void allowWhenNoDimensionsSet() {
try (MockedStatic<SystemWrapper> mocked = mockStatic(SystemWrapper.class)) {
mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");
MetricsUtils.defaultDimensionSet(new DimensionSet());
MetricsUtils.defaultDimensions(null);

requestHandler = new PowertoolsMetricsNoDimensionsHandler();
requestHandler.handleRequest("input", context);
Expand All @@ -321,7 +320,7 @@ public void exceptionWhenTooManyDimensionsSet() {
try (MockedStatic<SystemWrapper> mocked = mockStatic(SystemWrapper.class)) {
mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");

MetricsUtils.defaultDimensionSet(new DimensionSet());
MetricsUtils.defaultDimensions(null);

requestHandler = new PowertoolsMetricsTooManyDimensionsHandler();

Expand All @@ -336,7 +335,7 @@ public void metricsPublishedEvenHandlerThrowsException() {
try (MockedStatic<SystemWrapper> mocked = mockStatic(SystemWrapper.class)) {
mocked.when(() -> SystemWrapper.getenv("AWS_EMF_ENVIRONMENT")).thenReturn("Lambda");

MetricsUtils.defaultDimensionSet(new DimensionSet());
MetricsUtils.defaultDimensions(null);
requestHandler = new PowertoolsMetricsWithExceptionInHandler();

assertThatExceptionOfType(IllegalStateException.class)
Expand Down