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

[agents] Standardize on user-agent header set by (backend) agents #88

Closed
6 of 25 tasks
beniwohli opened this issue May 9, 2019 · 9 comments
Closed
6 of 25 tasks

Comments

@beniwohli
Copy link
Contributor

beniwohli commented May 9, 2019

Description of the issue

Currently, we have a hodgepodge of user-agent headers set by the backend agents:

Go-http-client/1.1
elastic-apm-node/2.6.0 elastic-apm-http-client/7.1.1
elasticapm-python/4.2.2
http.rb/4.1.1
java-agent/1.5.1-SNAPSHOT

(Thanks @graphaelli for compiling the list)

As a minimum, the user-agent should contain the language and the agent version. Preferably (IMO) it should also contain the product name (Elastic APM) in some form, which helps with discerning official agents from unofficial implementations.

Having both the language and the agent version is extremely helpful for debugging issues between agents and APM Servers.

I propose elasticapm-$language/$version.

There might also be some merit in including the name/version of the HTTP library used, e.g.

elasticapm-python/4.2.2 urllib3/1.24.0

The RUM agent is not included in the examples and vote, as it can't set the user-agent itself.

  • Once we agreed on a format, the agent development instructions should be updated

What we are voting on

Nothing yet, let's agree on a format first.

Vote

Agent Yes No Indifferent N/A Link to agent issue
.NET
elastic/apm-agent-dotnet#377
Go
elastic/apm-agent-go#527
Java
elastic/apm-agent-java#631
Node.js
elastic/apm-agent-nodejs#1223
Python
Already implemented
Ruby
elastic/apm-agent-ruby#416
@watson
Copy link
Contributor

watson commented May 9, 2019

Just a little background of the funky looking Node.js user-agent:

elastic-apm-node/2.6.0 elastic-apm-http-client/7.1.1

It's not uncommon in user-agent headers to concatenate lib/version strings like this when one library depends on another to make the actual HTTP request. In this case, the Node.js agent uses a separate elastic-apm-http-client library, so knowing both the version of the agent and the actual http lib can be useful - hence the two sets :)

The names used here are the actual names of the npm packages, where I guess we're just "lucky" that the Node.js agent has to have the word node in its name to not conflict it with the RUM JS agent.

The names are not hardcoded. I get the name of the npm package at runtime, so if someone forks the agent and releases it under another name, we'll see that new name. But if we feel it's best, I can change it to be hardcoded as elasticapm-node instead of grabbing the package name if it's important (not the module name, but the header of course).

I'm not sure this is possible with the other languages, but following the logic of chainging lib/version strings, we might actually add a node/<version> to the end to signify the Node.js version used (because we use the http client that ships with the Node.js runtime):

elastic-apm-node/2.6.0 elastic-apm-http-client/7.1.1 node/12.2.0

@axw
Copy link
Member

axw commented May 10, 2019

The names are not hardcoded. I get the name of the npm package at runtime, so if someone forks the agent and releases it under another name, we'll see that new name. But if we feel it's best, I can change it to be hardcoded as elasticapm-node instead of grabbing the package name if it's important (not the module name, but the header of course).

Probably best if we're consistent, in case we want to do some analysis on the headers. It wouldn't be hard to clean the data, but the less needed the better.

Do you think it would be useful to check the package name and only use "elasticapm-node" if it matches "elastic-apm-node"? That way if someone does change the package name for some reason, then we would still see it.

I'm not sure this is possible with the other languages, but following the logic of chainging lib/version strings, we might actually add a node/ to the end to signify the Node.js version used (because we use the http client that ships with the Node.js runtime):

👍 I think I'll do the same for the Go agent.

@mikker
Copy link
Contributor

mikker commented May 10, 2019

So are we agreeing on elastic-apm-language/version http-lib/version language/version, the latter two being optional?

@axw
Copy link
Member

axw commented May 10, 2019

I think we're agreeing on elasticapm-$language/$agent_version as a minimum, and then agents can append whatever they feel is useful for diagnosing issues for their agent.

@axw
Copy link
Member

axw commented Jul 17, 2019

Issues created for all backend agents, closing this now.

@watson
Copy link
Contributor

watson commented Aug 1, 2019

Sorry that I'm a little late to the party here, but the decision to change the name of the Node.js library in the user-agent to not match the name of the actual library seems confusing.

I appreciate where the incentive comes from (being able to have one common name across all official agents), but I also think it's useful information to know the name and version of the library being used.

So shouldn't elasticapm-{language} have been a prefix to the existing user-agent instead of replacing the name of the library being used?

E.g. the user-agent in Node.js agent has now been changed to elasticapm-node/{version}, but there is no such npm module with that version. I propose elasticapm-nodejs {library}/{version} instead. Would people be ok with that?

@axw
Copy link
Member

axw commented Aug 2, 2019

the user-agent in Node.js agent has now been changed to elasticapm-node/{version}, but there is no such npm module with that version

If we assume the module and language names are 1:1, then I don't see how it's confusing. I personally have been thinking of the user-agent as relating to the agent name/version, not the package or module name. The only reason it wouldn't be 1:1 is if someone forked the agent, but is that really something we need to deal with?

Go package paths include slashes, so it would look like elasticapm-go go.elastic.co/apm/v1.5.0 <maybe some other bits>. This would make parsing the common agent name and version more complicated.

Although it's duplicating the version, I think we should go with the common elasticapm-{agent.name}/{agent.version} prefix, and then after that add any module names and versions that are useful for problem diagnosis after that. Only the prefix is required. In the case of Node.js, you could tack on both the core module name and the transport module name. WDYT?

@watson
Copy link
Contributor

watson commented Aug 2, 2019

If we assume the module and language names are 1:1, then I don't see how it's confusing.

True, but they are not 1:1, right? Or maybe I misunderstood what you mean by module and language.

I personally have been thinking of the user-agent as relating to the agent name/version, not the package or module name.

I've always personally thought of it the opposite. I.e. that it's mainly a debugging tool, so I want to know the name and version of the libraries involved. But I'm 100% on board with the use-case of wanting to add the "official" agent name as well 👍

Go package paths include slashes, so it would look like elasticapm-go go.elastic.co/apm/v1.5.0 . This would make parsing the common agent name and version more complicated.

Good point, though I don't think parsing is going to be a normal use-case?

Although it's duplicating the version, I think we should go with the common elasticapm-{agent.name}/{agent.version} prefix, and then after that add any module names and versions that are useful for problem diagnosis after that. Only the prefix is required. In the case of Node.js, you could tack on both the core module name and the transport module name. WDYT?

Sounds good to me. My main concern was that the npm package name was no longer part of the Node user-agent string. Duplicating the version is not a big problem for me.

@axw
Copy link
Member

axw commented Aug 2, 2019

If we assume the module and language names are 1:1, then I don't see how it's confusing.

True, but they are not 1:1, right? Or maybe I misunderstood what you mean by module and language.

I meant that it's probably safe to assume that if the user-agent is "elasticapm-node", the agent module used is "elastic-apm-node"; and the versions are the same. Anyways, fine if we include both.

Good point, though I don't think parsing is going to be a normal use-case?

Hypothetically could be useful as a means of basic telemetry, without having to reach into the stream contents to pull out the agent metadata. Agreed that it's not a normal/typical use case though.

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

No branches or pull requests

4 participants