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

fix: panic err in counter metric creation #1295

Merged
merged 1 commit into from
Oct 6, 2023
Merged

Conversation

yashmehrotra
Copy link
Member

Fixes: #1289

@yashmehrotra yashmehrotra requested a review from moshloop October 5, 2023 15:10
@yashmehrotra yashmehrotra force-pushed the fix-gauge-metric branch 2 times, most recently from 1cc7668 to a87ae34 Compare October 5, 2023 19:15
func getWithEnvironment(ctx *context.Context, r *pkg.CheckResult) (*context.Context, error) {
// Convert result Data into JSON for templating
// as gomplate does not render child map data correctly
var rData map[string]any
Copy link
Member

Choose a reason for hiding this comment

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

Fixing this issue in #1288
I want to try and avoid unnecessary marshalling - if it needs to happen should be in gomplate.

}

case HistogramType:
if err := getOrCreateHistogram(m); err != nil {
result.ErrorMessage(fmt.Errorf("cannot create histogram %s with labels %v", m.Name, m.Labels))
logger.Errorf("cannot create histogram %s with labels %v: %w", m.Name, m.Labels, err)
Copy link
Member

Choose a reason for hiding this comment

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

Why would failing to create a metric, not fail the check ?

Copy link
Member Author

Choose a reason for hiding this comment

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

If the check passes but if there is an instrumentation error from our side, the check in itself still passes right ?

It might create false positives this way

@yashmehrotra yashmehrotra requested a review from moshloop October 6, 2023 07:37
@moshloop moshloop merged commit 63fe817 into master Oct 6, 2023
11 of 12 checks passed
@moshloop moshloop deleted the fix-gauge-metric branch December 14, 2023 08:57
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CRD status for http checks is always failed
2 participants