-
Notifications
You must be signed in to change notification settings - Fork 2.4k
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 pprof and create admin endpoint #1375
Conversation
looks good overall. Please get the build green ( |
b5b655f
to
f8c52be
Compare
Hm, I thought we had some bot that would add lint failures as comments on the appropriate lines. The build is failing with:
|
f8c52be
to
0bbd2a2
Compare
There are still some build errors, I will check it.. |
0bbd2a2
to
13414f0
Compare
Codecov Report
@@ Coverage Diff @@
## master #1375 +/- ##
=======================================
Coverage 99.82% 99.82%
=======================================
Files 173 173
Lines 8179 8179
=======================================
Hits 8165 8165
Misses 7 7
Partials 7 7 Continue to review full report at Codecov.
|
75de2d2
to
8695651
Compare
restarted crossdock integration test 3 times, it keeps failing, might be something to do with this PR. |
e.g. the renaming of the health port CLI flag |
The build is not stable for some reason. I've triggered it 3 times and got 3 different results. Crossdock was failing on my local machine 50% of the time even without my changes. |
8695651
to
eed044e
Compare
@konradgaluszka did you manage to get stable builds? I have restarted travis and it seems to be fine. @yurishkuro could this be merged? |
I am afraid build is still unstable. I was running the build locally on master branch (before my change) and still got failures in crossdock:
Do you have consistent runs for ./scripts/travis/build-crossdock.sh? |
This test seems to be unstable, not related to this PR:
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM, just need to either document the breaking change or provide a backwards compatible option.
I couldn't use aliasing in viper, because it does not work with reading from config: |
@konradgaluszka, looks like this last commit failed the DCO. Would you please amend it? |
Signed-off-by: Konrad Galuszka <konrad.galuszka.ctr@sabre.com> Add pprof endpoints (jaegertracing#1315)
96d8331
to
de60e44
Compare
Thanks! |
Resolves #1315
Refactored healthcheck package to have admin endpoint with both healthcheck and pprof endpoints