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

Remove Event field as it is causing issues with pre-trained source types #8039

Merged
merged 1 commit into from
Sep 18, 2020
Merged

Remove Event field as it is causing issues with pre-trained source types #8039

merged 1 commit into from
Sep 18, 2020

Conversation

samm-git
Copy link
Contributor

Required for all PRs:

  • Signed CLA.
  • [N/A] Associated README.md updated.
  • Has appropriate unit tests.

Description

I found that telegraf metrics export to splunk does not work on some of our environments. After all i found that reason was "event: metric" object in the json. To understand if it is needed here and why it works on some other environments i opened ticket to the vendor.

Summary of replies is:

  1. event field is not needed for the normal splunk metrics and it shoud work without it in all cases.
  2. On source types with pre-trained event types (in our case it was _json or collectd_http) splunk would fail if this field is exists, probably due to more strict json checking.

After this i decided to raise this PR to remove this field from the upstream. I tested it on my environments and it works fine.

@samm-git
Copy link
Contributor Author

Few quotes from support replies, just in case

I also get the following error:
07-23-2020 20:59:53.540 -0400 ERROR JsonLineBreaker - JSON StreamId:0 had parsing error:Unexpected character while looking for value: 'm' - data_source="disk", data_host="host_1.splunk.com", data_sourcetype="_json"
I also confirmed with senior engineers that the "event: metric" field is not necessary, also, as per the error above, it is causing an issue during parsing, which could be causing Splunk to drop the event.
My advice would be to leave a message in the feedback section located in the lowest part of the page, in the link where you found the curl command, mentioning that the documentation should be updated.

@sjwang90 sjwang90 added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Aug 28, 2020
@samm-git
Copy link
Contributor Author

@sjwang90 any chance to get this merged? We are using own fork due to that and it may affect other users.

Copy link
Contributor

@ssoroka ssoroka left a comment

Choose a reason for hiding this comment

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

seems reasonable. Thanks :)

@ssoroka ssoroka merged commit 191688c into influxdata:master Sep 18, 2020
idohalevi pushed a commit to idohalevi/telegraf that referenced this pull request Sep 29, 2020
…pes (influxdata#8039)

Co-authored-by: Oleksii Samorukov <oleksii_samorukov@mckinsey.com>
powersj added a commit to powersj/telegraf that referenced this pull request Jun 1, 2022
Contrary to influxdata#8039, splunk documentation does require the event tag with
metric value. This reverts that previous change.

fixes: influxdata#8761
arstercz pushed a commit to arstercz/telegraf that referenced this pull request Mar 5, 2023
…pes (influxdata#8039)

Co-authored-by: Oleksii Samorukov <oleksii_samorukov@mckinsey.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants