Skip to content

improv: update metrics interface to resemble other core utils - #60

Merged
to-mc merged 7 commits into
developfrom
improv/metrics_interface
Jun 6, 2020
Merged

improv: update metrics interface to resemble other core utils#60
to-mc merged 7 commits into
developfrom
improv/metrics_interface

Conversation

@to-mc

@to-mcto-mc commented Jun 5, 2020

Copy link
Copy Markdown
Contributor

Issue #, if available:#33

Description of changes:

Update Metrics to accept a "service" parameter to match Logger/Tracer. This parameter will be used as the metric namespace. Deprecated MetricManager.add_namespace env var.

BREAKING: Remove POWERTOOLS_METRICS_NAMESPACE, in favour of using the same environment variable as Logger/Tracer classes (POWERTOOLS_SERVICE_NAME).

@codecov-commenter

codecov-commenter commented Jun 5, 2020

Copy link
Copy Markdown

Codecov Report

Merging #60 into develop will increase coverage by 0.00%.
The diff coverage is 100.00%.

Impacted file tree graph

@@ Coverage Diff @@## develop #60 +/- ##
========================================
Coverage 99.38% 99.39% ========================================
Files 15 15 Lines 489 492 +3 Branches 44 44 ========================================
+ Hits 486 489 +3 
Misses 3 3 
Impacted FilesCoverage Δ
aws_lambda_powertools/metrics/base.py100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/metric.py100.00% <100.00%> (ø)
aws_lambda_powertools/metrics/metrics.py100.00% <100.00%> (ø)

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 5e5c2f5...8a4eab7. Read the comment docs.

@to-mc
to-mc requested a review from heitorlessaJune 6, 2020 09:39
@to-mcto-mc self-assigned this Jun 6, 2020

@heitorlessaheitorlessa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM - Minor typo only

Comment threaddocs/content/core/metrics.mdx Outdated
Comment threaddocs/content/index.mdx
@heitorlessa

Copy link
Copy Markdown
Contributor

Are you still planning to add the service as a dimension? or will that be in a separate PR?

Could you also change docstring in line 89 in metrics.py:

 **Lambda function using tracer and metrics decorators**
metrics = Metrics()

Thanks again, Tom!!

Tom McCarthyand others added 2 commits June 6, 2020 19:34
Co-authored-by: Heitor Lessa <heitor.lessa@hotmail.com>
@to-mc

to-mc commented Jun 6, 2020

Copy link
Copy Markdown
ContributorAuthor

Are you still planning to add the service as a dimension? or will that be in a separate PR?

I'm in two minds about adding a dimension with the service name. I'll create a separate issue/PR for that to discuss.

Could you also change docstring in line 89 in metrics.py:

 **Lambda function using tracer and metrics decorators**
metrics = Metrics()

Done!

Thanks again, Tom!!

@to-mc
to-mc requested a review from heitorlessaJune 6, 2020 18:47

@heitorlessaheitorlessa left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM!

@to-mc
to-mc merged commit 45f3436 into developJun 6, 2020
@to-mc
to-mc deleted the improv/metrics_interface branch June 6, 2020 18:54
heitorlessa referenced this pull request in heitorlessa/aws-lambda-powertools-python Jun 17, 2020
improv: update metrics interface to resemble other core utils
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

breaking-changeBreaking change

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@to-mc@codecov-commenter@heitorlessa