zipkin-reporter-java icon indicating copy to clipboard operation
zipkin-reporter-java copied to clipboard

include user agent in http reporting

Open jcchavezs opened this issue 6 years ago • 9 comments

zipkin server now exposes the user agent in the collector logs so it makes sense to include them here. A proposed format would be [component]/[version] ([client]) for example zipkin-go/0.12 or zipkin-php/1.4 (curl).

jcchavezs avatar Apr 29 '19 08:04 jcchavezs

probably we should prefer user agent formatting right? https://developer.mozilla.org/en-US/docs/Web/HTTP/Headers/User-Agent

User-Agent: <product> / <product-version> <comment>

Common format for web browsers:

User-Agent: Mozilla/<version> (<system-information>) <platform> (<platform-details>) <extensions>

Ex here's FF actually sending during this issue

Mozilla/5.0 (Macintosh; Intel Mac OS X 10.13; rv:66.0) Gecko/20100101 Firefox/66.0

Notable change is slash between thing and version of thing. Also, note there are multiple groups from least to most specific.

I'd guess that we could do something to tell it is zipkin api, if we like... or skip that and save bytes by not sending the prefix. However, with the prefix different language reporters can have common analysis (if that's the goal)

Ex. assuming this is the latest version of the zipkin api endpoint, we can say zipkin/2.1.0 or zipkin-api/2.1.0 after that give the version of the module in use

zipkin/2.1.0 zipkin-reporter-okhttp3/2.9.3

carefully cc'ing @apache/zipkin-committers

codefromthecrypt avatar Apr 30 '19 00:04 codefromthecrypt

cc @openzipkin/core preferences on agent format?

codefromthecrypt avatar Jul 04 '19 11:07 codefromthecrypt

I have a doubt here: I don't think the api endpoint is too important, actually the reporter could report send the information to any sort of endpoint (wrong v1 endpoint or intermediate agent). I would be more interested on set the format this reporter is sending, e.g. zipkin/v1 vs zipkin/v2. What do you think?

jcchavezs avatar Jul 12 '19 07:07 jcchavezs

first thought seems probably the format is less like user agent and more like content-type. also we have this in the url path...

On Fri, Jul 12, 2019 at 4:14 PM José Carlos Chávez [email protected] wrote:

I have a doubt here: I don't think the api endpoint is too important, actually the reporter could report send the information to any sort of endpoint (wrong v1 endpoint or intermediate agent). I would be more interested on set the format this reporter is sending, e.g. zipkin/v1 vs zipkin/v2. What do you think?

— You are receiving this because you are on a team that was mentioned. Reply to this email directly, view it on GitHub https://github.com/openzipkin/zipkin-reporter-java/issues/142?email_source=notifications&email_token=AAAPVVY7DRWLZP5UUJA4P4TP7AVNDA5CNFSM4HJBGIS2YY3PNVWWK3TUL52HS4DFVREXG43VMVBW63LNMVXHJKTDN5WW2ZLOORPWSZGODZY5GFY#issuecomment-510776087, or mute the thread https://github.com/notifications/unsubscribe-auth/AAAPVV26CIJSW2GIXCLUMJLP7AVNDANCNFSM4HJBGISQ .

codefromthecrypt avatar Jul 12 '19 07:07 codefromthecrypt

first thought seems probably the format is less like user agent and more like content-type

I agree. Perhaps we could consider adding this to the content type in a separate issue?

Ex. assuming this is the latest version of the zipkin api endpoint, we can say zipkin/2.1.0 or zipkin-api/2.1.0 after that give the version of the module in use

zipkin/2.1.0 zipkin-reporter-okhttp3/2.9.3

I like the second part. I'm not sure what the first part represents. Is that the Zipkin Server version?

shakuzen avatar Jul 30 '19 15:07 shakuzen

zipkin/2.1.0 zipkin-reporter-okhttp3/2.9.3

I like the second part. I'm not sure what the first part represents. Is that the Zipkin Server version?

zipkin core library version. It might be overly defensive, but in case people have version skew...

ps armeria sets user agent by default. ex requests made to elasticsearch from our server have user-agent=armeria/0.89.0

codefromthecrypt avatar Jul 31 '19 02:07 codefromthecrypt

zipkin core library version. It might be overly defensive, but in case people have version skew...

Makes sense. I'm on board with the proposed format, then. We just might need to take care with naming to make sure we can uniquely know which reporter implementation it is.

shakuzen avatar Jul 31 '19 03:07 shakuzen

let's go with the simplified armeria format I think because there's little chance of api drift and easier to deal with.

user-agent: armeria/0.89.0 so.. artifact/version or.. user-agent: zipkin-sender-okhttp3/2.10.2

@anuraaga @trustin @minwoox @huydx @kojilin make sense?

codefromthecrypt avatar Aug 13 '19 00:08 codefromthecrypt

SGTM

trustin avatar Aug 13 '19 01:08 trustin