[Metricbeat] [Statsd] Add support for Graphite series 1.1.0+ tags #39619

tehbooom · 2024-05-17T15:30:59Z

Proposed commit message

Added conditional to check if statsd metric contains , or ; and split accordingly

Checklist

My code follows the style guidelines of this project
I have commented my code, particularly in hard-to-understand areas
I have made corresponding changes to the documentation
I have made corresponding change to the default configuration files
I have added tests that prove my fix is effective or that my feature works
I have added an entry in CHANGELOG.next.asciidoc or CHANGELOG-developer.next.asciidoc.

Disruptive User Impact

Author's Checklist

[ ]

How to test this PR locally

Related issues

Closes [Metricbeat] Statsd Module does not support Graphite series 1.1.0+ #39473

Use cases

Screenshots

Logs

{
  "_index": ".ds-metricbeat-8.15.0-2024.05.17-000001",
  "_id": "iNaPho8Bf8HHEaVjdnAn",
  "_version": 1,
  "_score": 0,
  "_source": {
    "@timestamp": "2024-05-17T12:38:09.825Z",
    "service": {
      "type": "statsd"
    },
    "statsd": {
      "envoy_listener_worker_downstream_cx_active": {
        "value": 0
      }
    },
    "labels": {
      "envoy.worker_id": "10",
      "envoy.listener_address": "0.0.0.0_9001"
    },
    "ecs": {
      "version": "8.0.0"
    },
    "host": {
      "name": "mbp"
    },
    "agent": {
      "name": "mbp",
      "type": "metricbeat",
      "version": "8.15.0",
      "ephemeral_id": "41c29b63-8a18-4387-9879-a1b212dbcf8a",
      "id": "a934cbff-9d5b-4b73-84dd-363af4de9d23"
    },
    "metricset": {
      "name": "server"
    },
    "event": {
      "dataset": "statsd",
      "module": "statsd"
    }
  }
}

mergify · 2024-05-17T15:31:44Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @tehbooom? 🙏.
For such, you'll need to label your PR with:

The upcoming major version of the Elastic Stack
The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

backport-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

ritalwar · 2024-05-20T07:13:51Z

Can you also please update the changelog?

ritalwar

LGTM!

ritalwar · 2024-05-23T05:30:47Z

CHANGELOG.next.asciidoc

@@ -48,6 +48,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
 *Metricbeat*

 - Setting period for counter cache for Prometheus remote_write at least to 60sec {pull}38553[38553]
+- Added conditional to check if statsd metric contains , or ; and split accordingly {pull}39619[39619]


Suggested change

- Added conditional to check if statsd metric contains , or ; and split accordingly {pull}39619[39619]

- Added a conditional check to see if the statsd metric contains ',' or ';' and split accordingly. {pull}39619[39619]

ritalwar · 2024-05-23T05:39:07Z

Hi @aliabbas-elastic , Can we verify these changes? I've opened an issue for it here.
cc: @lalit-satapathy

ali786XI · 2024-05-23T06:04:34Z

Hi @aliabbas-elastic , Can we verify these changes? I've opened an #39681 for it here.

@ritalwar Ok I'll look into it

x-pack/metricbeat/module/statsd/server/data.go

Co-authored-by: subham sarkar <[email protected]>

x-pack/metricbeat/module/statsd/server/data.go

shmsr · 2024-05-23T15:51:02Z

CHANGELOG.next.asciidoc

@@ -48,6 +48,7 @@ https://github.com/elastic/beats/compare/v8.8.1\...main[Check the HEAD diff]
 *Metricbeat*

 - Setting period for counter cache for Prometheus remote_write at least to 60sec {pull}38553[38553]
+- Added a conditional check to see if the statsd metric contains ',' or ';' and split accordingly. {pull}39619[39619]


Suggested change

- Added a conditional check to see if the statsd metric contains ',' or ';' and split accordingly. {pull}39619[39619]

- Add support of Graphite series 1.1.0+ tagging extension for statsd module. {pull}39619[39619]

Also, I think we should update README. See an example: https://github.com/prometheus/statsd_exporter?tab=readme-ov-file#tagging-extensions

@ritalwar @tehbooom What do you think? Else, one has to go to code to figure it out to see what extensions are supported.

@ritalwar @shmsr How do we feel about adding this to the README?

[float] === Suported tag extensions The `statsd` module supports the following tags: https://docs.datadoghq.com/developers/dogstatsd/datagram_shell/?tab=metrics#the-dogstatsd-protocol[DogStatsD] `<metric name>:<value>|<type>|@samplerate|#<k>:<v>,<k>:<v>` https://github.com/influxdata/telegraf/blob/master/plugins/inputs/statsd/README.md#influx-statsd[InfluxDB] `<metric name>,<k>=<v>,<k>=<v>:<value>|<type>|@samplerate` https://graphite.readthedocs.io/en/latest/tags.html#graphite-tag-support[Graphite 1.1.x] `<metric name>;<k>=<v>;<k>=<v>:<value>|<type>|@samplerate`

Would look like this:

Perhaps the heading could be like:

"DogStatsD-style tags"

"InfluxDB-style tags"

and so on.

I'm fine with just listing the names, but we can add something like:

Supported Tag Extensions

Examples of tag styles supported by the statsd module:

DogStatsD
<metric name>:<value>|<type>|@samplerate|#<k>:<v>,<k>:<v>

InfluxDB
<metric name>,<k>=<v>,<k>=<v>:<value>|<type>|@samplerate

Graphite 1.1.x
<metric name>;<k>=<v>;<k>=<v>:<value>|<type>|@samplerate

shmsr · 2024-05-25T19:22:37Z

x-pack/metricbeat/module/statsd/server/data.go

+	// Metric tags could be separated by `,` or `;`
+	// We split here based on the separator


Suggested change

// Metric tags could be separated by `,` or `;`

// We split here based on the separator

// Metric tags could be separated by `,` or `;`, we split here based on the separator.

shmsr

LGTM. Let's wait for @ritalwar to approval as well and then we can merge.

ritalwar · 2024-05-29T07:09:28Z

@aliabbas-elastic, could you please do one round of testing as well so we can merge this?

ali786XI

Approving this PR but let me run one round of sanity tests for compatible metric formats. I'll update here soon

metricbeat/docs/modules/statsd.asciidoc

shmsr · 2024-05-29T07:37:03Z

Yes, let's wait for @aliabbas-elastic before merging

Co-authored-by: Aliabbas Attarwala <[email protected]>

added support for graphite tags

87f2025

tehbooom requested a review from a team as a code owner May 17, 2024 15:31

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label May 17, 2024

mergify bot assigned tehbooom May 17, 2024

added more unit tests

6998046

ycombinator added the Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team label May 17, 2024

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label May 17, 2024

shmsr added the enhancement label May 20, 2024

updated changelog

afd1eb2

ritalwar requested review from shmsr and ritalwar May 21, 2024 05:32

ritalwar reviewed May 23, 2024

View reviewed changes

ritalwar mentioned this pull request May 23, 2024

Testing use cases for updated StatsD module tag separation #39681

Closed

tehbooom added 2 commits May 23, 2024 08:12

Updated changelog

c98e920

Fixed spacing between headers

190d3a5

shmsr reviewed May 23, 2024

View reviewed changes

x-pack/metricbeat/module/statsd/server/data.go Outdated Show resolved Hide resolved

Update x-pack/metricbeat/module/statsd/server/data.go

7af5093

Co-authored-by: subham sarkar <[email protected]>

shmsr reviewed May 23, 2024

View reviewed changes

tehbooom added 5 commits May 23, 2024 13:45

Updated changelog

bdbbf83

Fixed code repetition and added comments

6f6a3c7

Updated README to show the valid extensions for the module

1229766

Updated module doc

4654853

Updated module documentation showing supported tags

82e71b8

shmsr reviewed May 25, 2024

View reviewed changes

Merge branch 'main' into statsd

8eb86e1

shmsr approved these changes May 29, 2024

View reviewed changes

ritalwar approved these changes May 29, 2024

View reviewed changes

ali786XI approved these changes May 29, 2024

View reviewed changes

metricbeat/docs/modules/statsd.asciidoc Outdated Show resolved Hide resolved

shmsr and others added 2 commits May 29, 2024 13:07

Update metricbeat/docs/modules/statsd.asciidoc

636dc35

Co-authored-by: Aliabbas Attarwala <[email protected]>

spelling

71b8904

tehbooom merged commit 61622ac into elastic:main May 30, 2024
26 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[Metricbeat] [Statsd] Add support for Graphite series 1.1.0+ tags #39619

[Metricbeat] [Statsd] Add support for Graphite series 1.1.0+ tags #39619

tehbooom commented May 17, 2024 •

edited by shmsr

Loading

mergify bot commented May 17, 2024

ritalwar commented May 20, 2024

ritalwar left a comment

ritalwar May 23, 2024

ritalwar commented May 23, 2024

ali786XI commented May 23, 2024

shmsr May 23, 2024

shmsr May 23, 2024

tehbooom May 23, 2024 •

edited

Loading

shmsr May 23, 2024

ritalwar May 24, 2024

shmsr May 25, 2024

shmsr left a comment •

edited

Loading

ritalwar commented May 29, 2024

ali786XI left a comment

shmsr commented May 29, 2024

	- Added conditional to check if statsd metric contains , or ; and split accordingly {pull}39619[39619]
	- Added a conditional check to see if the statsd metric contains ',' or ';' and split accordingly. {pull}39619[39619]

	- Added a conditional check to see if the statsd metric contains ',' or ';' and split accordingly. {pull}39619[39619]
	- Add support of Graphite series 1.1.0+ tagging extension for statsd module. {pull}39619[39619]

		// Metric tags could be separated by `,` or `;`
		// We split here based on the separator

[Metricbeat] [Statsd] Add support for Graphite series 1.1.0+ tags #39619

[Metricbeat] [Statsd] Add support for Graphite series 1.1.0+ tags #39619

Conversation

tehbooom commented May 17, 2024 • edited by shmsr Loading

Proposed commit message

Checklist

Disruptive User Impact

Author's Checklist

How to test this PR locally

Related issues

Use cases

Screenshots

Logs

mergify bot commented May 17, 2024

ritalwar commented May 20, 2024

ritalwar left a comment

Choose a reason for hiding this comment

ritalwar May 23, 2024

Choose a reason for hiding this comment

ritalwar commented May 23, 2024

ali786XI commented May 23, 2024

shmsr May 23, 2024

Choose a reason for hiding this comment

shmsr May 23, 2024

Choose a reason for hiding this comment

tehbooom May 23, 2024 • edited Loading

Choose a reason for hiding this comment

shmsr May 23, 2024

Choose a reason for hiding this comment

ritalwar May 24, 2024

Choose a reason for hiding this comment

shmsr May 25, 2024

Choose a reason for hiding this comment

shmsr left a comment • edited Loading

Choose a reason for hiding this comment

ritalwar commented May 29, 2024

ali786XI left a comment

Choose a reason for hiding this comment

shmsr commented May 29, 2024

tehbooom commented May 17, 2024 •

edited by shmsr

Loading

tehbooom May 23, 2024 •

edited

Loading

shmsr left a comment •

edited

Loading