Skip to content
This repository has been archived by the owner on Oct 9, 2023. It is now read-only.

Sanitize metric names before creation #114

Merged
merged 1 commit into from
Feb 9, 2022
Merged

Conversation

EngHabu
Copy link
Contributor

@EngHabu EngHabu commented Feb 9, 2022

Signed-off-by: Haytham Abuelfutuh [email protected]

Read then delete this section

- Make sure to use a concise title for the pull-request.

- Use #patch, #minor or #major in the pull-request title to bump the corresponding version. Otherwise, the patch version
will be bumped. More details

TL;DR

Please replace this text with a description of what this PR accomplishes.

Type

  • Bug Fix
  • Feature
  • Plugin

Are all requirements met?

  • Code completed
  • Smoke tested
  • Unit tests added
  • Code documentation added
  • Any pending items have an associated Issue

Complete description

How did you fix the bug, make the feature etc. Link to any design docs etc

Tracking Issue

Remove the 'fixes' keyword if there will be multiple PRs to fix the linked issue

fixes https://github.com/flyteorg/flyte/issues/

Follow-up issue

NA
OR
https://github.com/flyteorg/flyte/issues/

Signed-off-by: Haytham Abuelfutuh <[email protected]>
@codecov
Copy link

codecov bot commented Feb 9, 2022

Codecov Report

Merging #114 (73f3252) into master (df5879c) will increase coverage by 0.26%.
The diff coverage is 83.33%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #114      +/-   ##
==========================================
+ Coverage   67.86%   68.12%   +0.26%     
==========================================
  Files          66       66              
  Lines        3236     3247      +11     
==========================================
+ Hits         2196     2212      +16     
+ Misses        871      866       -5     
  Partials      169      169              
Flag Coverage Δ
unittests 66.71% <80.00%> (+0.30%) ⬆️

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

Impacted Files Coverage Δ
promutils/scope.go 87.38% <77.77%> (-0.47%) ⬇️
promutils/labeled/counter.go 42.85% <100.00%> (+2.11%) ⬆️
promutils/labeled/gauge.go 48.21% <100.00%> (+0.94%) ⬆️
promutils/labeled/stopwatch.go 67.56% <100.00%> (+0.90%) ⬆️
cache/auto_refresh.go 78.51% <0.00%> (+5.78%) ⬆️

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 df5879c...73f3252. Read the comment docs.

Copy link
Contributor

@hamersaw hamersaw left a comment

Choose a reason for hiding this comment

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

Are we concerned with any unit tests for sanitizing?

@kumare3 kumare3 merged commit 63db7a6 into master Feb 9, 2022
eapolinario pushed a commit that referenced this pull request Sep 6, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants