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(http_listener_v2): allows multiple paths and add path_tag #9529

Merged
merged 7 commits into from
Jul 27, 2021

Conversation

sumo-drosiek
Copy link
Contributor

Required for all PRs:

  • Updated associated README.md.
  • Wrote appropriate unit tests.

resolves #

Convert path configuration to paths to allow getting metrics on multiple endpoints. In addition adding path_tag
configuration option, so path can be included in defined tag name

@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 Jul 22, 2021
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Hey @sumo-drosiek thanks for this nice contribution! I added some minor comments in the code. Please keep the change backward compatible as users rely on the fact that a new telegraf still works with old config files.

plugins/inputs/http_listener_v2/README.md Outdated Show resolved Hide resolved
plugins/inputs/http_listener_v2/README.md Outdated Show resolved Hide resolved
plugins/inputs/http_listener_v2/http_listener_v2.go Outdated Show resolved Hide resolved
plugins/inputs/http_listener_v2/http_listener_v2.go Outdated Show resolved Hide resolved
plugins/inputs/http_listener_v2/http_listener_v2.go Outdated Show resolved Hide resolved
plugins/inputs/http_listener_v2/http_listener_v2.go Outdated Show resolved Hide resolved
plugins/inputs/http_listener_v2/http_listener_v2_test.go Outdated Show resolved Hide resolved
@srebhan srebhan self-assigned this Jul 22, 2021
@srebhan srebhan added the plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins label Jul 22, 2021
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Nice, leaves the path-tag issue to resolve. Thanks for working on this!

plugins/inputs/http_listener_v2/README.md Outdated Show resolved Hide resolved
plugins/inputs/http_listener_v2/http_listener_v2.go Outdated Show resolved Hide resolved
@sumo-drosiek sumo-drosiek changed the title feat(http_listener_v2)!: allows multiple paths and add path_tag feat(http_listener_v2): allows multiple paths and add path_tag Jul 23, 2021
Copy link
Member

@srebhan srebhan left a comment

Choose a reason for hiding this comment

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

Thanks for the quick cycles. The only thing left is to only add the Path if it is not empty and not yet in Paths (you create a duplication by default in your init()). Furthermore, there is a spurious config.yaml file that should not go into the repo I thinks.

config.yaml Outdated Show resolved Hide resolved
plugins/inputs/http_listener_v2/http_listener_v2.go Outdated Show resolved Hide resolved
plugins/inputs/http_listener_v2/http_listener_v2.go Outdated Show resolved Hide resolved
@sumo-drosiek
Copy link
Contributor Author

sumo-drosiek commented Jul 26, 2021

@srebhan I believe all issues have been resolved

sumo-drosiek pushed a commit to SumoLogic/telegraf that referenced this pull request Jul 26, 2021
sumo-drosiek pushed a commit to SumoLogic/telegraf that referenced this pull request Jul 26, 2021
Copy link
Member

@srebhan srebhan 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 to me. Thanks for this nice contribution @sumo-drosiek!

@srebhan srebhan added the ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review. label Jul 27, 2021
Copy link
Contributor

@sspaink sspaink left a comment

Choose a reason for hiding this comment

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

lgtm, thanks for the contribution!

@sspaink sspaink merged commit 348c18d into influxdata:master Jul 27, 2021
@sumo-drosiek sumo-drosiek deleted the drosiek-paths branch July 28, 2021 05:15
phemmer added a commit to phemmer/telegraf that referenced this pull request Aug 13, 2021
* origin/master: (183 commits)
  fix: CrateDB replace dots in tag keys with underscores (influxdata#9566)
  feat: Pull metrics from multiple AWS CloudWatch namespaces (influxdata#9386)
  fix: improve Clickhouse corner cases for empty recordset in aggregation queries, fix dictionaries behavior (influxdata#9401)
  fix(opcua): clean client on disconnect so that connect works cleanly (influxdata#9583)
  fix: Refactor ec2 init for config-api (influxdata#9576)
  fix: sort logs by timestamp before writing to Loki (influxdata#9571)
  fix: muting tests for udp_listener (influxdata#9578)
  fix: Do not return on disconnect to avoid breaking reconnect (influxdata#9524)
  fix: Fixing k8s nodes and pods parsing error (influxdata#9581)
  feat: OpenTelemetry output plugin (influxdata#9228)
  feat: Support AWS Web Identity Provider (influxdata#9411)
  fix: upgraded sensu/go to v2.9.0 (influxdata#9577)
  fix: Normalize unix socket path (influxdata#9554)
  docs: fix aws ec2 readme inconsistency (influxdata#9567)
  feat: Modbus Rtu over tcp enhancement (influxdata#9570)
  docs: information on new conventional commit format (influxdata#9573)
  docs: Add logo (influxdata#9574)
  docs: Adding links to net_irtt and dht_sensor external plugins (influxdata#9569)
  Upgrade hashicorp/consul/api to 1.9.1 (influxdata#9565)
  Update vmware/govmomi to v0.26.0 (influxdata#9552)
  Do not skip good quality nodes after a bad quality node is encountered (influxdata#9550)
  fix test so it hits a fake service (influxdata#9564)
  Update changelog
  Fix procstat plugin README to match sample config (influxdata#9553)
  Fix metrics reported as written but not actually written  (influxdata#9526)
  Prevent segfault in persistent volume claims (influxdata#9549)
  Update procstat to support cgroup globs & include systemd unit children (Copy of influxdata#7890) (influxdata#9488)
  Fix attempt to connect to an empty list of servers. (influxdata#9503)
  Fix handling bool in sql input plugin (influxdata#9540)
  Suricata alerts (influxdata#9322)
  Linter fixes for plugins/inputs/[fg]* (influxdata#9387)
  For Prometheus Input add ability to query Consul Service catalog (influxdata#5464)
  Support Landing page on Prometheus landing page (influxdata#8641)
  [Docs] Clarify tagging behavior (influxdata#9461)
  Change the timeout from all queries to per query (influxdata#9471)
  Attach the pod labels to the `kubernetes_pod_volume` & `kubernetes_pod_network` metrics. (influxdata#9438)
  feat(http_listener_v2): allows multiple paths and add path_tag (influxdata#9529)
  Bug Fix Snmp empty metric name (influxdata#9519)
  Worktable workfile stats (influxdata#8587)
  Update Go to v1.16.6 (influxdata#9542)
  ...
pmalek-sumo pushed a commit to SumoLogic/telegraf that referenced this pull request Oct 4, 2021
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 plugin/input 1. Request for new input plugins 2. Issues/PRs that are related to input plugins ready for final review This pull request has been reviewed and/or tested by multiple users and is ready for a final review.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants