Skip to content

feat: Add ClearAllDimensions Method #293

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 5 commits into from
Jun 12, 2023

Conversation

glynn1211
Copy link

@glynn1211 glynn1211 commented Jun 9, 2023

Please provide the issue number

Issue number: #292

Summary

Add a method to allow users to clear the existing Dimensions.

Changes

Added a ClearAllDimensions Method that will clear all dimensions which removes the default set "Service"

Checklist

Please leave checklist items unchecked if they do not apply to your change.

Is this a breaking change?

RFC issue number:

Checklist:

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

Acknowledgment

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

Disclaimer: We value your time and bandwidth. As such, any pull requests created on non-triaged issues might not be successful.

…he global DefaultDimensions and Instead checking if the passed variable is empty if so return so we don't delete the default, other wise set the Default to the passed ones
@auto-assign auto-assign bot requested a review from amirkaws June 9, 2023 13:44
@github-actions github-actions bot added feature New features or minor changes labels Jun 9, 2023
@codecov-commenter
Copy link

codecov-commenter commented Jun 9, 2023

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.16 🎉

Comparison is base (dba5e90) 69.09% compared to head (99e8e3b) 69.25%.

Additional details and impacted files
@@             Coverage Diff             @@
##           develop     #293      +/-   ##
===========================================
+ Coverage    69.09%   69.25%   +0.16%     
===========================================
  Files           79       79              
  Lines         3433     3448      +15     
===========================================
+ Hits          2372     2388      +16     
+ Misses        1061     1060       -1     
Flag Coverage Δ
unittests 69.25% <100.00%> (+0.16%) ⬆️

Flags with carried forward coverage won't be shown. Click here to find out more.

Impacted Files Coverage Δ
...aries/src/AWS.Lambda.Powertools.Metrics/Metrics.cs 85.07% <100.00%> (+0.69%) ⬆️
...rc/AWS.Lambda.Powertools.Metrics/Model/Metadata.cs 98.03% <100.00%> (+0.12%) ⬆️
...Lambda.Powertools.Metrics/Model/MetricDirective.cs 86.04% <100.00%> (+0.50%) ⬆️
....Lambda.Powertools.Metrics/Model/MetricsContext.cs 100.00% <100.00%> (ø)

... and 1 file with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

@hjgraca hjgraca added the area/metrics Core metrics utility label Jun 9, 2023
@hjgraca
Copy link
Contributor

hjgraca commented Jun 9, 2023

@glynn1211 thank you for submitting a PR.
I have left a comment on the issue.
We will review soon.

Copy link
Contributor

@hjgraca hjgraca left a comment

Choose a reason for hiding this comment

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

LGTM


var configurations = new Mock<IPowertoolsConfigurations>();

var logger = new Metrics(
Copy link
Contributor

Choose a reason for hiding this comment

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

Something we need to change later in all Metrics tests, the variable name should be metrics and not logger.

@hjgraca hjgraca assigned hjgraca and amirkaws and unassigned sliedig Jun 10, 2023
@amirkaws amirkaws merged commit 0a920b0 into aws-powertools:develop Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/metrics Core metrics utility feature New features or minor changes
Projects
Status: 🚀 Shipped
Development

Successfully merging this pull request may close these issues.

Feature request: Allow the default Service dimension to be removed from the Dimensions dictionary
6 participants