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

Update docs to mention brave and slf4j earlier #1466

Closed
bogdandrutu opened this issue Oct 11, 2019 · 7 comments · Fixed by #1614
Closed

Update docs to mention brave and slf4j earlier #1466

bogdandrutu opened this issue Oct 11, 2019 · 7 comments · Fixed by #1614

Comments

@bogdandrutu
Copy link

In the first sentence of this project description https://github.com/spring-cloud/spring-cloud-sleuth#spring-cloud-sleuth is a bit of a "lie".

Spring cloud sleuth is only available for "Brave" client users so that needs to be corrected and a proper description that states exactly that should be used:

Spring Cloud Sleuth is a distributed tracing plugin Spring Cloud that works only for Zipkin Brave users.
@bogdandrutu
Copy link
Author

Also everywhere on the website is stated that Sleuth is a distributed tracing solution for Spring Cloud, but this solution is only for Brave users.

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Oct 11, 2019 via email

@bogdandrutu
Copy link
Author

Yes, but user MUST use brave in their app if they want to have other Spans added. So it is a Brave implementation.

@bogdandrutu
Copy link
Author

bogdandrutu commented Oct 11, 2019

But this project can use any description they decide to sounds better for marketing. I am saying that I get questions why my application instrumented with other library does not work with Slueth. Because Slueth works only with "Brave".

@codefromthecrypt
Copy link
Contributor

codefromthecrypt commented Oct 12, 2019 via email

@bogdandrutu
Copy link
Author

A cleanup and even later mentioning the clear compatibilities is still a good step forward because I can point to that section when I get asked about other projects working with Sleuth. Also some advance section where you mention the incoming context format that you support would be good.

@spencergibb spencergibb changed the title "Miss leading" description of this project Update docs to mention brave and slf4j earlier Oct 12, 2019
@codefromthecrypt
Copy link
Contributor

@marcingrzejszczak I think in general this is a part of overdue cleanup as we still talk about "sr" "ss" etc. I may be wrong where I thought we don't support 1.x anymore, but I thought it isn't? Anyway :) Ideally most of this is deletion in nature, but we may want a larger issue and link this to it.

@bogdandrutu In questions about trace format support, this is likely another issue if we think about how the pull request could be addressed. We already have content in our docs about how to change the implementation.. this is used by spring-cloud-gcp for example to switch to google's headers, and we also have in zipkin-aws amazon's format. Probably it could be good in a separate pull request to clarify that sleuth supports out-of-box b3 and b3 single, and also link to other formats like what's in spring-cloud-gcp

My guess is that your questions are not about that, rather about the (what seems to be) stabilizing tracecontext format. As you probably know I did the first impl of that spec, as far as I know anywhere on gitub, back when I paired with @tylerbenson last year. It is important to not promote things until they are both relevant (end users asking for it, which hasn't happened yet) and until it is stable (maybe it is getting stable now). Anyway we can organize the propagation content to be more obvious that there are a few choices of trace formats today and what is out-of-box etc. For things about tracecontext in particular, we don't even have a single end user thumbsupping the change in brave, yet, or asking for it at all. I'm still cool to revise the impl but would appreciate a rewind of what changed in the last 1.5 years, and how to test if it is compatible. If you don't mind taking w3c off to this issue it would be less distracting to this repo. openzipkin/brave#693 When this becomes relevant, released, etc, of course we can link here

codefromthecrypt pushed a commit that referenced this issue Apr 9, 2020
While some overview is important, the direct implementation details of tracing
not only drifted since Sleuth 1.x, but also caused a complaint #1466

This deletes most documentation that applies to an abstraction lower than sleuth
and reworks existing docs to focus on features it enables. By doing so, we have
less to maintain and users will have a clearer idea about the relationship
between Sleuth, Zipkin and Brave. Also important is this uses updated screen
shots and is in general less cluttered than before.

Note: the relationship between Sleuth and Brave was already redone in #1603
Note: this also removes the pws app which is usually broken or out of date.

Fixes #1466
codefromthecrypt pushed a commit that referenced this issue Apr 9, 2020
While some overview is important, the direct implementation details of tracing
not only drifted since Sleuth 1.x, but also caused a complaint #1466

This deletes most documentation that applies to an abstraction lower than sleuth
and reworks existing docs to focus on features it enables. By doing so, we have
less to maintain and users will have a clearer idea about the relationship
between Sleuth, Zipkin and Brave. Also important is this uses updated screen
shots and is in general less cluttered than before.

Note: the relationship between Sleuth and Brave was already redone in #1603
Note: this also removes the pws app which is usually broken or out of date.

Fixes #1466
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants