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

Add Logstash input plugin (both version 5 and 6 support) #4910

Merged
merged 2 commits into from
Aug 21, 2019

Conversation

dmitryilyin
Copy link
Contributor

Add the input plugin to fetch metrics from the Logstash HTTP endpoints.
Both version 5 and 6 are supported.
Fetching process, jvm, events and plugins metrics.

Original developer:

  • Signed CLA.
  • Associated README.md updated.
  • Has appropriate unit tests.

Copy link
Contributor

@glinton glinton left a comment

Choose a reason for hiding this comment

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

Make sure you move the logstash README out of nginx_plus

@danielnelson
Copy link
Contributor

related #2702

@dmitryilyin dmitryilyin force-pushed the logstash branch 2 times, most recently from f57561c to da27a9b Compare October 23, 2018 18:51
@dmitryilyin
Copy link
Contributor Author

@glinton, oops, fixed

@dmitryilyin
Copy link
Contributor Author

@danielnelson, Yes, this implementations is based upon the initial development by @lkmcs but is significantly refactored.

@dmitryilyin dmitryilyin force-pushed the logstash branch 3 times, most recently from 1c2db81 to 1357892 Compare January 6, 2019 00:25
Add the input plugin to fetch metrics from the Logstash HTTP endpoints.
Both version 5 and 6 are supported.
Fetching process, jvm, events and plugins metrics.

Original developer:
* https://github.com/lkmcs/telegraf
Contributors:
* https://github.com/arkady-emelyanov/telegraf
* https://github.com/dmitryilyin/telegraf
@pvandervelde
Copy link

@dmitryilyin Is there a timeline to get this merged? It would be nice to be able to get performance metrics from Logstash

@langerma
Copy link

any news on that request?

@danielnelson danielnelson self-assigned this Mar 8, 2019
@langerma
Copy link

is there anything that is blocking this request?

@danielnelson
Copy link
Contributor

No, we will update as soon as we can, thanks for your patience.

Copy link
Contributor

@danielnelson danielnelson left a comment

Choose a reason for hiding this comment

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

Looks good, here are a few changes I would make:


# Editor files
*~
.idea
Copy link
Contributor

Choose a reason for hiding this comment

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

Revert changes to this file, but you can place these in your git core.excludesfile:

[core]
    excludesfile = ~/.gitignore

collect_queue_stats = true

## HTTP method
# method = "GET"
Copy link
Contributor

Choose a reason for hiding this comment

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

When would a user need to change the method?

* Tags:
- node_id
- node_name
- node_host
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's rename node_host to source to match with #4413.

# headers = {"X-Special-Header" = "Special-Value"}

## Override HTTP "Host" header
# host_header = "logstash.example.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

Is this just a general purpose HTTP option mostly for proxies?

}
accumulator.AddFields("logstash_events", flattener.Fields, tags)

if logstash.CollectPluginsStats {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's remove the collect_plugin_stats option because it isn't tied to an API call. Whenever we are collecting pipeline stats we will grab these, but it will still be possible to remove these metrics with the regular metric filtering (namedrop in this case).

}
}

if logstash.CollectQueueStats {
Copy link
Contributor

Choose a reason for hiding this comment

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

Let's also remove the collect_queue_stats option for the same reasons.

collect_jvm_stats = true

## Should the event pipelines statistics be gathered
collect_pipelines_stats = true
Copy link
Contributor

Choose a reason for hiding this comment

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

As an API style preference, let's switch to a list of items to collect:

multi_pipeline = true
collect_metrics = ["pipelines", "process", "jvm"]

# headers = {"X-Special-Header" = "Special-Value"}

## Override HTTP "Host" header
# host_header = "logstash.example.com"
Copy link
Contributor

Choose a reason for hiding this comment

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

Even though the Host header must be set in a special way, let's allow the user to set it as a normal header. This will be similar to the http output:

	for k, v := range h.Headers {
		if strings.ToLower(k) == "host" {
			req.Host = v
			continue
		}
		req.Header.Set(k, v)
	}

@danielnelson danielnelson mentioned this pull request Jul 29, 2019
3 tasks
@danielnelson danielnelson added this to the 1.12.0 milestone Jul 29, 2019
@danielnelson danielnelson mentioned this pull request Aug 3, 2019
3 tasks
@danielnelson
Copy link
Contributor

danielnelson commented Aug 21, 2019

Will do a followup PR with the changes from my review.

#6299

@danielnelson danielnelson merged commit 0217403 into influxdata:master Aug 21, 2019
@danielnelson danielnelson mentioned this pull request Aug 21, 2019
3 tasks
bitcharmer pushed a commit to bitcharmer/telegraf that referenced this pull request Oct 18, 2019
athoune pushed a commit to bearstech/telegraf that referenced this pull request Apr 17, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants