Skip to content
This repository has been archived by the owner on May 23, 2024. It is now read-only.

consider add more verbosity to error for http span sender #596

Closed
Dentrax opened this issue Sep 10, 2021 · 0 comments
Closed

consider add more verbosity to error for http span sender #596

Dentrax opened this issue Sep 10, 2021 · 0 comments
Assignees

Comments

@Dentrax
Copy link

Dentrax commented Sep 10, 2021

Requirement - what kind of business use case are you trying to solve?

It's a bit hard to distinguish why collector return non-200 errors, like as following:

prometheus-prometheus-operator-kube-p-prometheus-0 thanos-sidecar {"caller":"logger.go:22","level":"error","msg":"error reporting span \"/thanos.Store/Info\": error from collector: 404","ts":"2021-09-10T18:02:45.713894993Z"}

We could not figure it out which endpoint it tries to send for, with what method and maybe body?

It had better to error out: with this request: [POST] http://foo/bar:8080 i got 404 instead of i got 404.

Problem - what in Jaeger blocks you from solving the requirement?

This line throws the error from collector: %d errors.

This function actually.

Proposal - what do you suggest to solve the problem or improve the existing situation?

  • We can add a debug log here before sending the request: req.String() maybe?

  • We can enrich the verbosity of this error:

if resp.StatusCode >= http.StatusBadRequest {
 return fmt.Errorf("error from collector: request: %s, got status: %d", req.String(), resp.StatusCode)
}

Wdyt?

@Dentrax Dentrax changed the title consider add more verbosity for span sender (debug log) consider add more verbosity to error for http span sender Sep 10, 2021
Dentrax added a commit to Dentrax/jaeger-client-go that referenced this issue Sep 11, 2021
Fixes jaegertracing#596

Signed-off-by: Furkan <furkan.turkal@trendyol.com>
Dentrax added a commit to Dentrax/jaeger-client-go that referenced this issue Sep 13, 2021
Fixes jaegertracing#596

Signed-off-by: Furkan <furkan.turkal@trendyol.com>
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 a pull request may close this issue.

3 participants