Skip to content

Conversation

@jcarres-mdsol
Copy link
Collaborator

@jcarres-mdsol jcarres-mdsol commented Dec 16, 2016

The faraday middleware created ids and cleaned up after him which is good, except when there were no ids to begin with.
This fixes the issue and allows to always clean up after him.
Also added more classes because I like classes

@ykitamura-mdsol @adriancole @jfeltesse-mdsol @ssteeg-mdsol

if @sampled_as_boolean
@logger && @logger.warn("Using a boolean in the Sampled header is deprecated. Consider setting sampled_as_boolean to false")
end
Trace.sample_rate = @sample_rate
Copy link
Contributor

Choose a reason for hiding this comment

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

👍

:zookeeper, :log_tracing,
:annotate_plugin, :filter_plugin, :whitelist_plugin].each do |method|
it "can set and read configuration values for #{method}" do
it 'can set and read configuration values for #{method}' do
Copy link
Contributor

Choose a reason for hiding this comment

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

should be " ~ "

@JordiPolo JordiPolo force-pushed the fix_faraday_setting_ids branch from af9e7be to 52b1a2d Compare December 16, 2016 08:52
@JordiPolo JordiPolo force-pushed the fix_faraday_setting_ids branch from 52b1a2d to 06f1ecd Compare December 16, 2016 08:53
Copy link
Member

@codefromthecrypt codefromthecrypt left a comment

Choose a reason for hiding this comment

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

easier to see what's going on. Do you consider TraceContainer and TraceGenerator internal details or a part of the api? do they invalidate or warrant anything in the README?

@jcarres-mdsol
Copy link
Collaborator Author

Still internal details, give it a new couple of versions to mature, just in case.

@jcarres-mdsol jcarres-mdsol merged commit 84672f7 into openzipkin:master Dec 19, 2016
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.

3 participants