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 additional tag/log to rules Manager Eval trace span. #7708

Merged
merged 1 commit into from
Aug 6, 2020
Merged

Add additional tag/log to rules Manager Eval trace span. #7708

merged 1 commit into from
Aug 6, 2020

Conversation

cstyan
Copy link
Member

@cstyan cstyan commented Jul 31, 2020

Adds a tag to the span that says whether the evaluation succeeded, and a log line to the span with the error message in the result of failure.

Signed-off-by: Callum Styan callumstyan@gmail.com

@roidelapluie
Copy link
Member

should we simply set the "standard" error=true on failed evaluations?

@cstyan
Copy link
Member Author

cstyan commented Jul 31, 2020

Fine by me, the name of the tag isn't support important to me and if there's a standard I'm not aware of it. However I do need both the status (failed/succeeded) and the error message log.

@roidelapluie
Copy link
Member

roidelapluie commented Jul 31, 2020

Fine by me, the name of the tag isn't support important to me and if there's a standard I'm not aware of it. However I do need both the status (failed/succeeded) and the error message log.

As the jaeger UI offers error=true in its UI exemples by default I have always assumed it was kinda standard

@roidelapluie
Copy link
Member

error=true is detected by jaeger ui as well and displayed with a red tag.

@cstyan
Copy link
Member Author

cstyan commented Aug 5, 2020

error=true is detected by jaeger ui as well and displayed with a red tag. yep, you're right, I only just learned about this the day I opened this PR, @roidelapluie PTAL

Signed-off-by: Callum Styan <callumstyan@gmail.com>
@cstyan cstyan merged commit c7a17f6 into prometheus:master Aug 6, 2020
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