Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Track errors when recording spans #290

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

ChimeraCoder
Copy link
Contributor

@ChimeraCoder ChimeraCoder commented Oct 16, 2017

Summary

If we're encountering an error when recording a span, we want to know about it! This helps us catch situations like dropping spans due to the channel buffer being full.

Motivation

Test plan

Rollout/monitoring/revert plan

r? @asf-stripe
cc @stripe/observability

@stripe-ci
Copy link

Gerald Rule: Copy Observability on Veneur and Unilog pull requests

cc @stripe/observability
cc @stripe/observability-stripe

Copy link
Contributor

@asf-stripe asf-stripe left a comment

Choose a reason for hiding this comment

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

I think only one blocker on this. Otherwise: AWESOME

cl.statsMtx.Lock()
defer cl.statsMtx.Unlock()
tags := []string{fmt.Sprintf("error:%s", err.Error())}
cl.stats.Incr("veneur.trace.record.errors_total", tags, 0.1)
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the metrics prefix that we set on our statsd client will cause the metric to be reported as veneur.veneur.trace.<...>. I wonder if SetErrorStats should have a sister method to set the metric name, too.

@@ -61,6 +65,18 @@ func (c *Client) run(ctx context.Context) {
}
}

type IncrClient interface {
Copy link
Contributor

Choose a reason for hiding this comment

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

Can/should this interface get docs?

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants