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

prometheus input plugin: mistaken delete #9408

Closed
wujianhanshu opened this issue Jun 22, 2021 · 8 comments · Fixed by #9497
Closed

prometheus input plugin: mistaken delete #9408

wujianhanshu opened this issue Jun 22, 2021 · 8 comments · Fixed by #9497
Assignees
Labels
area/prometheus bug unexpected problem or unintended behavior regression something that used to work, but is now broken

Comments

@wujianhanshu
Copy link

Relevant telegraf.conf:

[[inputs.prometheus]]
      metric_version = 1
      pod_scrape_scope = "node"
      monitor_kubernetes_pods = true
      insecure_skip_verify = true
      pod_scrape_interval = 1
      bearer_token = "/run/secrets/kubernetes.io/serviceaccount/token"

Mistaken info:

Telegraf: v1.8.3-v1.19.0

https://github.com/influxdata/telegraf/pull/8937/files#diff-dcebbfefbef8e986af6172ea7f96ab8b6d0b62aac383a7fa3f7bc1df5d06f2e0

func (p *Prometheus) cAdvisor(ctx context.Context) error {
...
client.SetHeaders(req.Header) // mistaken delete
...
}

@wujianhanshu wujianhanshu added the bug unexpected problem or unintended behavior label Jun 22, 2021
@wujianhanshu
Copy link
Author

@SebastianThorn

@reimda
Copy link
Contributor

reimda commented Jun 23, 2021

Hi @wujianhanshu, is this causing telegraf to fail? Could you give more detail about what happens when you run v1.19.0?

@sspaink Could you look into why this client.SetHeaders(req.Header) line was removed? It looks like it happened when you migrated telegraf to kubernetes/client-go a few months ago.

@reimda reimda added the regression something that used to work, but is now broken label Jun 23, 2021
@sspaink
Copy link
Contributor

sspaink commented Jun 23, 2021

@reimda The new client used in the library "kubernetes/client-go" didn't have have an equivalent to the HTTP client field that the old *k8s.Client had which was being used to send the HTTP request. So instead I updated the code to use the standard HTTP library to make the request resp, err := httpClient.Do(req) (link to code) and because the new code is using the req object directly it already has the correct header.

@wujianhanshu is the request to update the cadvisor pod list not working for you?

@sspaink sspaink added the waiting for response waiting for response from contributor label Jun 24, 2021
@wujianhanshu
Copy link
Author

@sspaink
yes, update the cadvisor pod list not working.
error message:
2021-06-29T02:29:33Z E! [inputs.prometheus] Unable to monitor pods with node scrape scope: Error initially updating pod list: Error when making request for pod list with status 401 Unauthorized`

the new code is using the req object directly it already has the correct header.

httpClient := http.Client{}
req, err := http.NewRequest("GET", podsURL, nil)

In the above code, there is no correct header information.
You can try it, 100 percent.

@telegraf-tiger telegraf-tiger bot removed the waiting for response waiting for response from contributor label Jun 29, 2021
@sspaink
Copy link
Contributor

sspaink commented Jul 6, 2021

@gracewehner could you perhaps help with this issue? Reaching out because you helped contribute the feature to support listing from cadvisor originally (#8762), and I don't have a good way to test this. It does seem the code is now missing the step to set the header properly like it did with the old library (link to old way), but it looks like the new kubernetes library it might be the right way to use this Pods list function. but it isn't obvious, I could try to submit a pull request with a best effort but would take sometime for me to figure out how to test it properly. Maybe @wujianhanshu or @gracewehner you have a recommendation for an easy way to recreate an environment that uses this feature? Thanks!

@imranismail
Copy link
Contributor

This fixes it #9497

@srebhan
Copy link
Member

srebhan commented Jul 14, 2021

@wujianhanshu can you confirm that #9497 fixes the problem?

@wujianhanshu
Copy link
Author

he fixed the problem,it was excellent. @srebhan

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/prometheus bug unexpected problem or unintended behavior regression something that used to work, but is now broken
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants