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

Update monitoring mixin #3331

Merged
merged 1 commit into from
Oct 21, 2021
Merged

Update monitoring mixin #3331

merged 1 commit into from
Oct 21, 2021

Conversation

jpkrohling
Copy link
Contributor

@jpkrohling jpkrohling commented Oct 19, 2021

Closes #2956

Signed-off-by: Juraci Paixão Kröhling juraci@kroehling.de

@jpkrohling jpkrohling requested a review from a team as a code owner October 19, 2021 12:13
@jpkrohling jpkrohling requested a review from yurishkuro October 19, 2021 12:13
@jpkrohling
Copy link
Contributor Author

@gouthamve, as the original author of this mixin, would you like to review it?
@esnible, would you be open to trying this out as well? I tested it locally and seems fine, but would be good to have a confirmation from you as well.

@codecov
Copy link

codecov bot commented Oct 19, 2021

Codecov Report

Merging #3331 (e3251e5) into master (a240917) will decrease coverage by 0.05%.
The diff coverage is n/a.

❗ Current head e3251e5 differs from pull request most recent head 4dcb887. Consider uploading reports for the commit 4dcb887 to get more accurate results
Impacted file tree graph

@@            Coverage Diff             @@
##           master    #3331      +/-   ##
==========================================
- Coverage   96.01%   95.96%   -0.06%     
==========================================
  Files         259      259              
  Lines       15434    15434              
==========================================
- Hits        14819    14811       -8     
- Misses        522      528       +6     
- Partials       93       95       +2     
Impacted Files Coverage Δ
pkg/config/tlscfg/cert_watcher.go 92.63% <0.00%> (-2.11%) ⬇️
cmd/query/app/server.go 94.11% <0.00%> (-1.48%) ⬇️
cmd/query/app/static_handler.go 95.80% <0.00%> (-1.20%) ⬇️
plugin/storage/integration/integration.go 78.88% <0.00%> (-0.40%) ⬇️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update a240917...4dcb887. Read the comment docs.

gouthamve
gouthamve previously approved these changes Oct 20, 2021
Copy link
Contributor

@gouthamve gouthamve left a comment

Choose a reason for hiding this comment

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

LGTM

@jpkrohling
Copy link
Contributor Author

@jaegertracing/jaeger-maintainers , would one of you please review?

albertteoh
albertteoh previously approved these changes Oct 21, 2021
Copy link
Contributor

@albertteoh albertteoh left a comment

Choose a reason for hiding this comment

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

LGTM, just a couple of suggestions.

@jpkrohling jpkrohling dismissed stale reviews from albertteoh and gouthamve via 20974ca October 21, 2021 09:36
@jpkrohling jpkrohling requested a review from albertteoh October 21, 2021 09:45
Signed-off-by: Juraci Paixão Kröhling <juraci@kroehling.de>
@jpkrohling jpkrohling merged commit 7e6caaf into jaegertracing:master Oct 21, 2021
# 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.

Jaeger grafana mixin is broken
3 participants