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

Add missing metrics #14036

Merged
merged 1 commit into from
Feb 28, 2023
Merged

Conversation

FlorentClarret
Copy link
Member

@FlorentClarret FlorentClarret commented Feb 27, 2023

What does this PR do?

  • Fix some metric names which were copy/pasted
  • Remove a duplicated entry in the metric maps

Motivation

  • Spotted working on a support case

Additional Notes

Review checklist (to be filled by reviewers)

  • Feature or bugfix MUST have appropriate tests (unit, integration, e2e)
  • PR title must be written as a CHANGELOG entry (see why)
  • Files changes must correspond to the primary purpose of the PR as described in the title (small unrelated changes should have their own PR)
  • PR must have changelog/ and integration/ labels attached
  • If the PR doesn't need to be tested during QA, please add a qa/skip-qa label.

@ghost ghost added the integration/envoy label Feb 27, 2023
@FlorentClarret FlorentClarret force-pushed the florentclarret/envoy/duplicate-metrics branch from 36f4a6e to 2903475 Compare February 27, 2023 12:46
@FlorentClarret FlorentClarret changed the title Fix some metric names Add missing metrics Feb 27, 2023
@FlorentClarret FlorentClarret force-pushed the florentclarret/envoy/duplicate-metrics branch from 2903475 to b505d3d Compare February 27, 2023 13:24
@codecov
Copy link

codecov bot commented Feb 27, 2023

Codecov Report

Merging #14036 (36892a8) into master (79dbdd1) will increase coverage by 0.00%.
The diff coverage is 100.00%.

Flag Coverage Δ
envoy 94.13% <100.00%> (+0.04%) ⬆️

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

@FlorentClarret FlorentClarret marked this pull request as ready for review February 27, 2023 13:42
@FlorentClarret FlorentClarret requested a review from a team as a code owner February 27, 2023 13:42
@FlorentClarret FlorentClarret force-pushed the florentclarret/envoy/duplicate-metrics branch 2 times, most recently from d2d3c2d to 9425ab0 Compare February 27, 2023 13:59
@FlorentClarret FlorentClarret force-pushed the florentclarret/envoy/duplicate-metrics branch from 9425ab0 to 36892a8 Compare February 27, 2023 14:41
@@ -108,7 +108,6 @@
'envoy_cluster_upstream_rq_rx_reset': 'cluster.upstream_rq_rx_reset',
'envoy_cluster_upstream_rq_time': 'cluster.upstream_rq_time',
'envoy_cluster_upstream_rq_timeout': 'cluster.upstream_rq_timeout',
'envoy_cluster_upstream_rq': 'cluster.upstream_rq',
Copy link
Contributor

Choose a reason for hiding this comment

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

A bit surprised by this one, I would expect some linting rule to catch this (duplicate dict key) 🤔 .

Copy link
Member Author

@FlorentClarret FlorentClarret Feb 28, 2023

Choose a reason for hiding this comment

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

To be honest, I missed that one too, my IDE linter caught it 😅

Edit: Seems like flake8 does not catch this kind of errors if the values are identical: https://flake8.pycqa.org/en/5.0.4/user/error-codes.html

@FlorentClarret FlorentClarret merged commit 34bebaf into master Feb 28, 2023
@FlorentClarret FlorentClarret deleted the florentclarret/envoy/duplicate-metrics branch February 28, 2023 12:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants