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

Starlark script for renaming prometheus remote write metrics #9074

Merged
merged 8 commits into from Mar 31, 2021

Conversation

helenosheaa
Copy link
Member

@helenosheaa helenosheaa commented Mar 30, 2021

Adds script to Starlark examples and to the readme of the prometheus remote write parser.
Enables alignment with InfluxDB v1.8 prometheus remote write spec outlined here

The provided Starlark script renames the measurement name using the fieldname (which holds the Prometheus metric name), and renames the fieldname to value.

Relates to #9038

Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

🤝 ✅ CLA has been signed. Thank you!

@telegraf-tiger telegraf-tiger bot added the feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin label Mar 30, 2021
Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

plugins/parsers/prometheusremotewrite/README.md Outdated Show resolved Hide resolved
plugins/parsers/prometheusremotewrite/README.md Outdated Show resolved Hide resolved
plugins/parsers/prometheusremotewrite/README.md Outdated Show resolved Hide resolved
plugins/parsers/prometheusremotewrite/README.md Outdated Show resolved Hide resolved
Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@sjwang90 sjwang90 left a comment

Choose a reason for hiding this comment

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

awesome!

Comment on lines 10 to 14
for k, v in metric.fields.items():
metric.name = k
metric.fields["value"] = v
metric.fields.pop(k)
return metric
Copy link
Contributor

Choose a reason for hiding this comment

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

if we're going to recommend this to everyone, we should include a metric.name guard in this function to look for the metric name, and either handle the case where there are multiple fields (they can't all be the metric name), or specify in the description and file name that this is specifically for prometheus.

Copy link
Member Author

Choose a reason for hiding this comment

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

Added a guard to check it's promtheus_remote_write. I'd put in the other example a namepass but probably makes more sense to have it in there. I've added in to the name and description that it's for promtheus remote write where there is only one field as that is the use case.

@@ -237,6 +237,7 @@ def apply(metric):
- [multiple metrics from json array](/plugins/processors/starlark/testdata/multiple_metrics_with_json.star) - Builds a new metric from each element of a json array then returns all the created metrics.
- [custom error](/plugins/processors/starlark/testdata/fail.star) - Return a custom error with [fail](https://docs.bazel.build/versions/master/skylark/lib/globals.html#fail).
- [compare with previous metric](/plugins/processors/starlark/testdata/compare_metrics.star) - Compare the current metric with the previous one using the shared state.
- [rename measurement](/plugins/processors/starlark/testdata/rename.star) - Rename measurement name with fieldname and rename fieldname to value.
Copy link
Contributor

Choose a reason for hiding this comment

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

there is already a rename.star, double check file name

Copy link
Member Author

Choose a reason for hiding this comment

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

updated to rename_prometheus_remote_write


- Use the [Starlark processor](https://github.com/influxdata/telegraf/blob/master/plugins/processors/starlark/README.md) to rename the measurement name to the fieldname and rename the fieldname to value. Use namepass to only apply the script to prometheus_remote_write metrics

- Example script:
Copy link
Contributor

Choose a reason for hiding this comment

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

maybe just link to the script so we don't have to keep the two docs in sync.

Copy link
Contributor

Choose a reason for hiding this comment

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

yeah i suggested otherwise so people could just copy and paste from the readme but one extra click isn't a big deal

Copy link
Member Author

Choose a reason for hiding this comment

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

changed to a link

Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

Copy link
Contributor

@telegraf-tiger telegraf-tiger bot left a comment

Choose a reason for hiding this comment

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

@helenosheaa helenosheaa merged commit 885bf27 into master Mar 31, 2021
@helenosheaa helenosheaa deleted the rename-prw-starlark branch March 31, 2021 19:08
@disfluxly
Copy link

@helenosheaa - Thanks so much for adding this in!

@helenosheaa
Copy link
Member Author

helenosheaa commented Apr 1, 2021

@disfluxly no problem! I hope this helps for your use case. I'll close the related issue but if there are any issues feel free to reopen.

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.

None yet

4 participants