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(inputs.sflow): Add default source tag #14112

Closed

Conversation

Hipska
Copy link
Contributor

@Hipska Hipska commented Oct 16, 2023

Required for all PRs

Related to #8011 and #4413

Add the source tag to standardise the sflow input.

@telegraf-tiger telegraf-tiger bot added feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins labels Oct 16, 2023
@Hipska Hipska added the area/network New plugins or feature relating to Network Monitoring label Oct 16, 2023
@srebhan
Copy link
Member

srebhan commented Oct 17, 2023

@Hipska while I love the PR, the issue is that you double the cardinality (at least for InfluxDB) users with this change. Can you please add an option to switch between the existing tag and the new source tag!

@Hipska
Copy link
Contributor Author

Hipska commented Oct 17, 2023

I did follow METRIC_FORMAT_CHANGES doc, it is not considered to be unsafe (whose need such an option).

@powersj
Copy link
Contributor

powersj commented Oct 17, 2023

it is not considered to be unsafe (whose need such an option).

While adding a tag is not specifically listed as "unsafe" it is listed as "be careful with
cardinality". The addition of this tag is not strictly necessary. In order to prevent issues who may already be at a limit of cardinality for whatever reason, then it should follow the unsafe change guidelines:

If your change has the potential to seriously affect existing users, the change must be opt-in.

@srebhan srebhan self-assigned this Oct 18, 2023
@powersj
Copy link
Contributor

powersj commented Oct 25, 2023

@Hipska same question here -do you want to make it a doc change to suggest using "source" or close this?

@Hipska
Copy link
Contributor Author

Hipska commented Oct 26, 2023

Well, if the idea is to deprecate this plugin, then its best to just close this PR I guess..

@Hipska Hipska closed this Oct 26, 2023
@Hipska Hipska deleted the feat/inputs/sflow/source_tag branch November 7, 2023 15:18
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area/network New plugins or feature relating to Network Monitoring feat Improvement on an existing feature such as adding a new setting/mode to an existing plugin plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants