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

Report zipkin spans regardless of goal version. #7915

Merged
merged 2 commits into from Jun 24, 2019

Conversation

Projects
None yet
4 participants
@hrfuller
Copy link
Contributor

commented Jun 20, 2019

Problem

fixes #7386

Goals which use the v2 engine were not exporting zipkin spans because the code doing the exporting was inside the context of the legacy graph, which isn't used during v2 engine goal execution.

Solution

Moved the zipkin bulk export code to the local pants runner so that it gets called regardless of which engine is run.

Result

Users can now see spans / timings from engine workunits displayed through zipkin dash for both v1 and v2 goals.

@stuhood stuhood requested review from cattibrie and illicitonion Jun 20, 2019

@stuhood
Copy link
Member

left a comment

Thanks! Looks good.

Will merge on green CI.

@cattibrie
Copy link
Contributor

left a comment

Thank you for this PR!!!
I am still in the process of reviewing. Will leave other comments later.

@illicitonion
Copy link
Contributor

left a comment

Looks good! Thanks!

Show resolved Hide resolved tests/python/pants_test/reporting/test_reporting_integration.py Outdated
Show resolved Hide resolved src/python/pants/bin/local_pants_runner.py Outdated

@hrfuller hrfuller force-pushed the hrfuller:hfuller/fix-zipkin-metrics branch from 21d3dce to 68419d1 Jun 21, 2019

@illicitonion illicitonion merged commit af9e2ad into pantsbuild:master Jun 24, 2019

1 check failed

continuous-integration/travis-ci/pr The Travis CI build could not complete due to an error
Details

@hrfuller hrfuller deleted the hrfuller:hfuller/fix-zipkin-metrics branch Jun 24, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.