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

feat: logstash #11421

Closed
wants to merge 3 commits into from
Closed

feat: logstash #11421

wants to merge 3 commits into from

Conversation

HeikoSchlittermann
Copy link
Contributor

In a large scale environment Telegraf sends data to Logstash instances. For some reason the Logstash receivers are not efficient in "unwrapping" the hash to get the metrics array.

Our change provides a new config option "json_logstash_support". This option defaults to "false", so the behaviour of existing instances is not changed. Setting this option to "true" drops the outermost envelope and sends the data just as an array with all metrics.

logstash support off (the default): { metrics => [ m1, m2, m3, ... ] }
logstash support on : [m1, m2, m3, ...]

This option is used in pre-production already and allows us more throughput when sending the metrics.

HeikoSchlittermann and others added 3 commits June 29, 2022 23:33
The default JSON output looks like:

  {
    metrics: [
      {
      …
      },
      {
      …
      },
    ],
  }

Logstash receivers don't seem to work well with high volume of this and
work better with

  [
      {
      …
      },
      {
      …
      },
  ]
@HeikoSchlittermann HeikoSchlittermann changed the title Feat/logstash feat: logstash Jun 30, 2022
@srebhan srebhan self-assigned this Jul 5, 2022
@srebhan srebhan added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin area/json json and json_v2 parser/serialiser related plugin/serializer labels Jul 5, 2022
@srebhan
Copy link
Member

srebhan commented Jul 5, 2022

Hey @HeikoSchlittermann thanks for the PR! As this is actually not necessarily logstash related, we should maybe avoid the option and make it a new serializer? One option would be to have a json_v2 serializer that omits the metrics tag, just like what you get in your implementation with json_logstash_support = true. Alternatively, we could make the jsonata serializer to output this "new" format by default. What do you think?

@powersj powersj added the waiting for response waiting for response from contributor label Jul 7, 2022
@HeikoSchlittermann
Copy link
Contributor Author

Hey @srebhan, sorry for the delayed response.
I'm fine with changing the name to whatever suits better. And from what I see about JSONata, it looks promising, so yes, we can integrate it as default format there.

I'll check how to do it, or - if you know already how to do it, feel free to change our PR or cherry-pick whatever you want (but please keep the author names if possible)

@telegraf-tiger telegraf-tiger bot removed the waiting for response waiting for response from contributor label Jul 12, 2022
@srebhan
Copy link
Member

srebhan commented Jul 27, 2022

@HeikoSchlittermann I don't think you need any code change here. Please try to use the http output as before with json and add the following to the plugin:

json_transformation = "metrics"

Can you please give it a try?

This probably only works with master or a recent nightly build as #11251 didn't make it to a release yet...

@fiete201
Copy link
Contributor

Hi @srebhan thanks for your feedback. I start testing your suggesting now.

@fiete201
Copy link
Contributor

fiete201 commented Sep 8, 2022

Hi @srebhan,
testing looks good so far. I just had to edit http.go:

From 3a4bc7d844839e1326c56943e2759d45d6b88bd4 Mon Sep 17 00:00:00 2001
From: Fritz Reichwald <reichwald@b1-systems.de>
Date: Thu, 8 Sep 2022 11:40:19 +0200
Subject: [PATCH] Fix: add json_transformation as variable to HTTP struct

---
 plugins/outputs/http/http.go | 1 +
 1 file changed, 1 insertion(+)

diff --git a/plugins/outputs/http/http.go b/plugins/outputs/http/http.go
index 668e7c04b..20243321f 100644
--- a/plugins/outputs/http/http.go
+++ b/plugins/outputs/http/http.go
@@ -52,6 +52,7 @@ type HTTP struct {
        UseBatchFormat          bool              `toml:"use_batch_format"`
        AwsService              string            `toml:"aws_service"`
        NonRetryableStatusCodes []int             `toml:"non_retryable_statuscodes"`
+       JsonTransformation      string            `toml:"json_transformation"`
        httpconfig.HTTPClientConfig
        Log telegraf.Logger `toml:"-"`
 
-- 
2.37.3

To avoid the error:

2022-09-08T09:13:14Z E! [telegraf] Error running agent: Error loading config file test.config: plugin outputs.http: line 27: configuration specified the fields ["json_transformation"], but they weren't used

The config I used to test:

[global_tags]
[agent]
  interval = "10s"
  round_interval = true
  metric_batch_size = 1000
  metric_buffer_limit = 10000
  collection_jitter = "0s"
  flush_interval = "10s"
  flush_jitter = "0s"
  precision = ""
  hostname = ""
  omit_hostname = false
[[inputs.disk]]
  ignore_fs = ["tmpfs", "devtmpfs", "devfs", "iso9660", "overlay", "aufs", "squashfs", "ext4"]

[[ outputs.http ]]
  url = "http://localhost:8080/telegraf"
  timeout = "10s"
  method = "POST"
  data_format = "json"
  use_batch_format = true
  json_transformation = "metrics"
  [ outputs.http.headers ]
    Content-Type = "application/json;charset=utf-8"

Was my change at the correct place? Or would you prefer to add "json_transformation" to one of the cases in missingTomlField in config.go?

@srebhan
Copy link
Member

srebhan commented Sep 8, 2022

@HeikoSchlittermann thanks for testing!

testing looks good so far. I just had to edit http.go [...]

This is the wrong place as it would require all output plugins to add that option. In my PR #11251 I forgot to add the new option to the missingTOMLField() function. The option belongs there. Do you mind adding it with your PR?

@fiete201
Copy link
Contributor

fiete201 commented Sep 8, 2022

Haha, I had it there in the first place but was not sure if this is the correct one. Will prepare it that way.

@fiete201
Copy link
Contributor

Thanks for your patience @srebhan
I tested successfully and can confirm that using json_transformation = "metrics" solves our problem.
Hence this PR is no longer needed.
I will ask @HeikoSchlittermann to please close it.

Best regards
Fritz

@HeikoSchlittermann
Copy link
Contributor Author

I'm closing this PR (and remove the branches on our side), as with the advent of 1.24 our patch won't be necessary anymore (we're successfully using the current master branch). The jsonata feature provides us a great solution.

Thanks to @srebhan and the Telegraf team for your development.

@HeikoSchlittermann HeikoSchlittermann deleted the feat/logstash branch September 14, 2022 12:26
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/json json and json_v2 parser/serialiser related feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/serializer
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants