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

Set an X-Telegraf-Version Header in HTTP outputs #3911

Closed
jregovic opened this issue Mar 20, 2018 · 14 comments
Closed

Set an X-Telegraf-Version Header in HTTP outputs #3911

jregovic opened this issue Mar 20, 2018 · 14 comments
Labels
feature request Requests for new plugin and for new features to existing plugins
Milestone

Comments

@jregovic
Copy link

Feature Request

Encode the Agent version in X-Telegraf-Version

Proposal:

Make the default configuration for telegraf set a header for X-Telegraf-Version

Current behavior:

There is no header.

Desired behavior:

Telegraf supplies its version in HTTP requests

Use case: [Why is this important (helps with prioritizing requests)]

This can be useful when trying to verify the versions running in large environments and determine whether or not updates have been successful. It could also allow for suppressing some data from a bad version. Having the agent set an immutable value will provide an accurate value.

@danielnelson
Copy link
Contributor

I think we should use the User-Agent header instead of a custom header, something like:

User-Agent: Telegraf/1.5.3

@danielnelson danielnelson added the feature request Requests for new plugin and for new features to existing plugins label Mar 20, 2018
@kerams
Copy link
Contributor

kerams commented Mar 23, 2018

How would you read that value? By sending HTTP output to a custom proxy service in front of the database?

@danielnelson
Copy link
Contributor

I think this is one way it could be used, but also this issue doesn't specifically mention the InfluxDB output. To have proper behavior we need to follow rfc2616 on all of our HTTP clients:

The User-Agent request-header field contains information about the
user agent originating the request. This is for statistical purposes,
the tracing of protocol violations, and automated recognition of user
agents for the sake of tailoring responses to avoid particular user
agent limitations. User agents SHOULD include this field with
requests.

This gets me thinking that maybe we should add version as a tag to the internal input.

@kerams
Copy link
Contributor

kerams commented Mar 24, 2018

This gets me thinking that maybe we should add version as a tag to the internal input.

That's clever and would be useable straight away. The redis input does the same thing, except the version is a field, not a tag.

@jregovic
Copy link
Author

Adding the version tot he internal plugin might be good, but I don't know that I am necessarily interested in collecting data in all of the agents like that just to get the version.

I'd want the data in an immutable header because I might, on occasion, want to be able to know what agents are on which versions, but I only need it when we need to verify an upgrade.

I can run a tcpdump and look at the headers if I need to.

@danielnelson
Copy link
Contributor

@jregovic You probably know about this, but until this is implemented you can use the http_headers or user_agent setting in the InfluxDB output to do this manually.

@KorwinS
Copy link

KorwinS commented Jun 28, 2018

This gets me thinking that maybe we should add version as a tag to the internal input.

I wound up on this issue when looking for that feature. That would be a great addition for keeping track of which agents we have in our environment.

@kevinconaway
Copy link
Contributor

kevinconaway commented Oct 4, 2018

I took a look at this to see if this could be something that I could tackle.

The central problem here is that the version information is isolated in the telegraf main module, it isn't accessible from any of the plugin implementations.

This code base is fairly new to me so this may be off base but one idea that I had for this would be to add something like an AgentInformation struct to the Config struct. This struct would contain at a minimum the version information. It could be populated by the main module when the Config is being set up.

The plugin interface bootstrap methods (ServiceInput#Start, ServiceOutput#Connect, etc) could be modified to accept the Config struct which would allow the plugin implementations to use that information.

The downsides with that approach is that it would be a breaking change to the plugin interfaces and would require all of them to be changed. It may also be a concern to leak the Config information in to the plugin implementations which could encourage misuse.

@glinton do you have thoughts on the best way that this could be handled?

@kevinconaway
Copy link
Contributor

Another option could be to have the linker write the version information in to selfstat instead of writing it in to the main module. Everything could then query selfstat for the version information as needed.

@glinton
Copy link
Contributor

glinton commented Oct 5, 2018

All great points, to which I'm going to refer back to @danielnelson for design specifics, but it seems like centralizing the version so it's available for all plugins (both input and output), not just the http output as originally proposed.

@danielnelson
Copy link
Contributor

This seems like the right approach but lets add a function, or functions, to the internal package to retrieve the version.

@kevinconaway
Copy link
Contributor

to the internal package to retrieve the version.

So should we modify the link step to write the version values to the internal module instead and then expose them via a function?

@danielnelson
Copy link
Contributor

Hmm, maybe we should continue to write them to main.version in the linker step since this seems to be the convention, but have cmd/telegraf/telegraf.go set them to internal, so in internal/internal.go we would add:

func SetVersion(version string)
func GetVersion() string

kevinconaway pushed a commit to kevinconaway/telegraf that referenced this issue Oct 8, 2018
kevinconaway pushed a commit to kevinconaway/telegraf that referenced this issue Oct 8, 2018
kevinconaway added a commit to kevinconaway/telegraf that referenced this issue Oct 8, 2018
kevinconaway added a commit to kevinconaway/telegraf that referenced this issue Oct 10, 2018
kevinconaway added a commit to kevinconaway/telegraf that referenced this issue Oct 11, 2018
@danielnelson danielnelson added this to the 1.9.0 milestone Nov 12, 2018
@danielnelson
Copy link
Contributor

Added in #4828.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature request Requests for new plugin and for new features to existing plugins
Projects
None yet
Development

No branches or pull requests

6 participants