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

Remove --zipkin-trace-v2 option #10184

Merged
merged 3 commits into from Jun 29, 2020

Conversation

gshuflin
Copy link
Contributor

This commit removes the --zipkin-trace-v2 option and tests using it. In the v2 world, we will implement zipkin tracing (if necessary) with a plugin that consumes streaming workunits. This also allows us to remove the get_workunits rust FFI function, which was only used to implement this functionality, and no longer have to pass around the zipkin tracing boolean in rust code.

Copy link
Contributor

@Eric-Arellano Eric-Arellano left a comment

Choose a reason for hiding this comment

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

Yay!

Copy link
Sponsor Member

@stuhood stuhood left a comment

Choose a reason for hiding this comment

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

Thanks!

@@ -51,68 +50,6 @@ def test_zipkin_reporter_with_zero_sample_rate(self):
num_of_traces = len(ZipkinHandler.traces)
self.assertEqual(num_of_traces, 0)

Copy link
Sponsor Member

Choose a reason for hiding this comment

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

The test above this looks dead as well?

@@ -94,13 +94,6 @@ def register_options(cls, register):
default=100.0,
help="Rate at which to sample Zipkin traces. Value 0.0 - 100.0.",
)
Copy link
Sponsor Member

@stuhood stuhood Jun 27, 2020

Choose a reason for hiding this comment

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

I expect that there is a lot more code in here related to zipkin that can be removed.

@gshuflin gshuflin merged commit 1b79666 into pantsbuild:master Jun 29, 2020
@gshuflin gshuflin deleted the remove_zipkin_trace branch June 29, 2020 20:36
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants