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.

Sign up for GitHub

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

Jump to bottom

Prometheus client: fix lock copy + add traces #3664

Merged
merged 1 commit into from Feb 3, 2021
Merged

Prometheus client: fix lock copy + add traces #3664

merged 1 commit into from Feb 3, 2021

Conversation

jotak
Copy link
Contributor

@jotak jotak commented Feb 2, 2021

  • Do not copy prom's default round tripper as it contains a lock
  • Add trace-level logs on prom queries

@jotak
Copy link
Contributor Author

jotak commented Feb 2, 2021

Relates to #3660

@jotak jotak mentioned this pull request Feb 2, 2021
Closed
@jotak
Copy link
Contributor Author

jotak commented Feb 2, 2021

side note: the lock copy issue was visible in lint reports. We should make them more visible.
There's a few other lint warnings in the code base but nothing that critical.

@jotak
Copy link
Contributor Author

jotak commented Feb 2, 2021

@lucasponce @jshaughn despite it does not fix #3660 I think it's still important to merge

@jotak jotak added this to In Review in Sprint 52 via automation Feb 2, 2021
@jotak jotak self-assigned this Feb 2, 2021
@jotak
Prometheus client: fix lock copy + add traces
8d56312 
- Do not copy prom's default round tripper as it contains a lock
- Add trace-level logs on prom queries
@jotak
Copy link
Contributor Author

jotak commented Feb 2, 2021

(+ 1 test added as part of investigations)

jshaughn
jshaughn approved these changes Feb 2, 2021
View reviewed changes
Copy link
Collaborator

@jshaughn jshaughn left a comment

Choose a reason for hiding this comment

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

@jotak good find/change.

@jotak jotak merged commit b3a7b5f into kiali:master Feb 3, 2021
Sprint 52 automation moved this from In Review to Done Feb 3, 2021
@ghost ghost added this to the v1.30.0 milestone Feb 3, 2021
@lucasponce
Copy link
Contributor

lucasponce commented Feb 3, 2021 •
edited

Please @jotak let's also backport the potential fixes for performance to the v1.29.x branch which will target Istio 1.9

xeviknal pushed a commit to xeviknal/kiali that referenced this pull request Feb 3, 2021
@jotak @xeviknal
Prometheus client: fix lock copy + add traces (kiali#3664)
c11329c 
- Do not copy prom's default round tripper as it contains a lock
- Add trace-level logs on prom queries
@xeviknal xeviknal mentioned this pull request Feb 3, 2021
Merged
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jshaughn jshaughn jshaughn approved these changes

@lucasponce lucasponce Awaiting requested review from lucasponce

Assignees

@jotak jotak

Labels
None yet
Projects
No open projects
Sprint 52
  
Done
Milestone
v1.30.0
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@jotak @lucasponce @jshaughn