Fix event mapping implementation for statsd module #36925

ritalwar · 2023-10-21T17:35:34Z

Bug

Proposed commit message

The root cause of the problem is the implementation of the event mapping method.

The data periodically sent by Airflow is not limited to a single DAG; it can encompass information from all currently running DAGs. This collective data is gathered into a group and subsequently processed to generate an event. The issue was traced back to a for loop used to iterate through all the key-value pairs within this group to create a single event. Because the data originates from multiple DAGs, creating a single event resulted in the data from one DAG overwriting the data from another. This led to the chaotic mixing of metrics, causing the problem.

The new implementation matches the approach used in the statsd_exporter. Now, each key-value pair and its associated attributes are treated as individual events. As a result, each key-value pair is processed to create distinct events, with only the related fields present in each event. This modification effectively resolves the issue and offers greater flexibility when using controls and filters to generate visualizations for associated data.

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.

How to test this PR locally

Enable the statsd module.
When testing any module, such as airflow, configure the statsd mapping.
Verify the data and mappings in Elasticsearch.

Related issues

Closes https://github.com/elastic/sdh-beats/issues/3932

Screenshots

mergify · 2023-10-21T17:36:09Z

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @ritalwar? 🙏.
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

elasticmachine · 2023-10-21T18:04:13Z

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

Duration: 56 min 6 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

/test : Re-trigger the build.
/package : Generate the packages and run the E2E tests.
/beats-tester : Run the installation tests with beats-tester.
run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

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

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

lalit-satapathy · 2023-10-23T06:17:40Z

Hi @ritalwar,
Can we summarise the the issue/problem and solution being done here (also an old document vs new document will help)?

ritalwar · 2023-10-24T06:47:48Z

Issue details:
Using statsd.mappings to link fields with labels and attributes, it was noticed that fields containing multiple <label_placeholders> were causing jumbled metrics data in Elasticsearch. This led to incorrect values being included in the event JSON. This issue arose while utilizing the Metricbeat statsd module to send metrics from Apache Airflow to statsd.

Root Cause:
During the code debugging process, we pinpointed the root cause of this problem to be the implementation of the event mapping method.

Here are the specifics: The data periodically sent by Airflow is not limited to a single DAG; it can encompass information from all currently running DAGs. This collective data is gathered into a group and subsequently processed to generate an event. The issue was traced back to a for loop used to iterate through all the key-value pairs within this group to create a single event. Because the data originates from multiple DAGs, creating a single event resulted in the data from one DAG overwriting the data from another. This led to the chaotic mixing of metrics, causing the problem at hand.

Solution:

We've updated the implementation to match the approach used in the statsd exporter. Now, each key-value pair and its associated attributes are treated as individual events. As a result, each key-value pair is processed to create distinct events, with only the related fields present in each event. This modification effectively resolves the issue and offers greater flexibility when using controls and filters to generate visualizations for related data.

ritalwar · 2023-10-24T06:54:28Z

For this configuration:

- metric: 'dag_processing.last_duration.<dag_file>'
        labels:
          - attr: dag_file
            field: dag_file
        value:
          field: dag_last_duration

Document generated before the change, with all data combined into a single event:

And after change, document featuring only one related field and its attributes within a single event:

ritalwar · 2023-10-24T06:56:06Z

Sample dashboard Examples:

ali786XI · 2023-10-24T11:01:21Z

Testing Details

Test Setup

Used a mock server to send custom metrics to StatsD module and verify the results

Test Case Scenarios

Test Case Summaries	Status
Verify that the metrics are getting mapped correctly in case of a single placeholder value	Pass 💚
Verify that the metrics are getting mapped correctly in case of multiple placeholders	Pass 💚
Check that the metrics are getting mapped correctly in case of no placeholders	Pass 💚
Check that the metrics are received individually in single documents in case of same placeholders	Pass 💚
Check that the labels are also getting mapped correctly	Pass 💚
Check that there are no errors related to the mappings while running the beats module	Pass 💚

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

devamanv · 2023-10-25T05:04:11Z

Just left a minor comment, all other changes look good to me.

x-pack/metricbeat/module/airflow/statsd/_meta/data.json

…statsd_mapping_3932

agithomas · 2023-10-27T06:20:10Z

The change looks good to me. However, as discussed, i would recommend to have it tested with Graphite for additional test coverage, if time permits.

@aliabbas-elastic ,
can you please validate the data grouping logic post the change?

agithomas

LGTM!

ritalwar · 2023-10-27T06:35:34Z

@aliabbas-elastic , can you please validate the data grouping logic post the change?

Yes, we can check if statsd.mappings work well with Graphite metrics and if it's logical to have separate events in there too.

ali786XI · 2023-10-27T06:42:22Z

@aliabbas-elastic ,
can you please validate the data grouping logic post the change?

Sure @agithomas I am looking into it.

ali786XI · 2023-10-27T12:01:49Z

Brought up Graphite with default metrics in it

Defined mappings for the same in metricbeat.yml and everything seems logical in implementation as metrics are getting separated in individual documents.

Refer one sample_event.json for carbon.aggregator.2aeab3d714e5-a.destinations.activeConnections

Corresponding mapping for the metric in metricbeat.yml

  - metric: carbon.aggregator.<agent_id>.destinations.activeConnections
    labels:
      - attr: agent_id
        field: agent_id
    value:
      field: aggregator_activeConnections

Sample Event for the same:-

{
  "_index": ".ds-metricbeat-8.12.0-2023.10.27-000001",
  "_id": "D-6tcIsBueTgBUaS5yc4",
  "_version": 1,
  "_score": 0,
  "_source": {
    "@timestamp": "2023-10-27T10:28:46.030Z",
    "service": {
      "type": "statsd"
    },
    "statsd": {
      "agent_id": "2aeab3d714e5-a",
      "aggregator_activeConnections": {
        "value": 3
      }
    },
    "labels": {},
    "ecs": {
      "version": "8.0.0"
    },
    "host": {
      "name": "co7"
    },
    "agent": {
      "name": "co7",
      "type": "metricbeat",
      "version": "8.12.0",
      "ephemeral_id": "127a9de0-290b-4033-b209-fe29aa3b2a13",
      "id": "78f2c980-468e-44b3-9a85-4c47fe79c50f"
    },
    "event": {
      "module": "statsd",
      "dataset": "statsd"
    },
    "metricset": {
      "name": "server"
    }
  },
  "fields": {
    "statsd.agent_id": [
      "2aeab3d714e5-a"
    ],
    "statsd.aggregator_activeConnections.value": [
      3
    ],
    "kibana_stats.timestamp": [
      "2023-10-27T10:28:46.030Z"
    ],
    "agent.hostname": [
      "co7"
    ],
    "metricset.name": [
      "server"
    ],
    "service.type": [
      "statsd"
    ],
    "agent.type": [
      "metricbeat"
    ],
    "@timestamp": [
      "2023-10-27T10:28:46.030Z"
    ],
    "logstash_stats.timestamp": [
      "2023-10-27T10:28:46.030Z"
    ],
    "agent.id": [
      "78f2c980-468e-44b3-9a85-4c47fe79c50f"
    ],
    "event.module": [
      "statsd"
    ],
    "ecs.version": [
      "8.0.0"
    ],
    "agent.ephemeral_id": [
      "127a9de0-290b-4033-b209-fe29aa3b2a13"
    ],
    "agent.name": [
      "co7"
    ],
    "agent.version": [
      "8.12.0"
    ],
    "host.name": [
      "co7"
    ],
    "beats_state.timestamp": [
      "2023-10-27T10:28:46.030Z"
    ],
    "event.dataset": [
      "statsd"
    ],
    "beats_state.state.host.name": [
      "co7"
    ],
    "timestamp": [
      "2023-10-27T10:28:46.030Z"
    ]
  }
}

cc:- @agithomas @ritalwar

tommyers-elastic · 2023-10-27T08:56:01Z

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

-	for idx, tagGroup := range groups {
-
-		mapstrTags := mapstr.M{}
+	// If there are no groups, return an empty slice.


tiny nit: the comment does not match the code - we aren't returning an empty slice here.

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

tommyers-elastic · 2023-10-27T14:48:48Z

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

+			}
+			events = append(events, &mb.Event{
+				MetricSetFields: ms,
+				RootFields:      mapstr.M{"labels": mapstrTags},


do we need to worry about increasing storage cost by duplicating the tags across many more metric documents?

These tags aren't always there. We're not adding them ourselves; they are optional. In fact, I couldn't find them in the case of Airflow, so there is no extra overhead involved. More about StatsD tags in general, as they are supported by every StatsD implementation, such as Datadog and Atlassian. Same we are supporting this here.

tommyers-elastic · 2023-10-27T14:50:10Z

x-pack/metricbeat/module/airflow/statsd/data_test.go

 	<-done

+	assert.Len(t, events, 2)


does this test fail without the new changes in this PR?

Missed running the test before making this change, but I've run it now and made the necessary updates(the tags needed to be identical for both the events to test this change). Thanks!

* Fix eventmapping implementation for statsd module. (cherry picked from commit adcd4b0)

* Fix eventmapping implementation for statsd module.

Update eventmapping implementation for statsd module

c734cf2

ritalwar added the bug label Oct 21, 2023

ritalwar requested a review from a team as a code owner October 21, 2023 17:35

botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Oct 21, 2023

ritalwar self-assigned this Oct 21, 2023

ritalwar added the Team:Obs-InfraObs Label for the Observability Infrastructure Monitoring team label Oct 21, 2023

botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Oct 21, 2023

Fix test cases

ca489d5

shmsr changed the title ~~Update eventmapping implementation for statsd module~~ Update event mapping implementation for statsd module Oct 21, 2023

ritalwar added 2 commits October 21, 2023 23:42

Fixing Linting issue

69d2262

Update Changelog.asciidoc

f1e8e37

ritalwar changed the title ~~Update event mapping implementation for statsd module~~ Fix event mapping implementation for statsd module Oct 21, 2023

devamanv reviewed Oct 22, 2023

View reviewed changes

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

devamanv reviewed Oct 22, 2023

View reviewed changes

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

devamanv requested a review from a team October 22, 2023 19:18

Update review comments

5cd8440

shmsr reviewed Oct 23, 2023

View reviewed changes

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

Update server.go

800fbf5

shmsr approved these changes Oct 23, 2023

View reviewed changes

ritalwar requested a review from devamanv October 24, 2023 07:07

ritalwar added 2 commits October 24, 2023 21:15

Update data_test.go

ef5837b

Update data_test.go

4fc3100

devamanv reviewed Oct 24, 2023

View reviewed changes

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

devamanv approved these changes Oct 25, 2023

View reviewed changes

agithomas reviewed Oct 25, 2023

View reviewed changes

x-pack/metricbeat/module/airflow/statsd/_meta/data.json Show resolved Hide resolved

ritalwar and others added 3 commits October 25, 2023 13:08

Merge branch 'main' into statsd_mapping_3932

1a2f7c9

Update comments for getEvents

e21fba9

Merge branch 'statsd_mapping_3932' of github.com:ritalwar/beats into …

6b738dd

…statsd_mapping_3932

lalit-satapathy requested a review from tommyers-elastic October 26, 2023 04:54

agithomas approved these changes Oct 27, 2023

View reviewed changes

tommyers-elastic approved these changes Oct 27, 2023

View reviewed changes

Address review comments

b2cb507

ritalwar force-pushed the statsd_mapping_3932 branch from f46ecaa to b2cb507 Compare October 30, 2023 05:58

ritalwar added 2 commits October 30, 2023 11:40

Update test

74eb3ae

update data_test.go

b4a9cd2

ritalwar added backport-v8.11.0 Automated backport with mergify backport and removed backport labels Oct 30, 2023

ritalwar merged commit adcd4b0 into elastic:main Oct 30, 2023
8 checks passed

mergify bot pushed a commit that referenced this pull request Oct 30, 2023

Fix event mapping implementation for statsd module (#36925)

a512874

* Fix eventmapping implementation for statsd module. (cherry picked from commit adcd4b0)

mergify bot mentioned this pull request Oct 30, 2023

[8.11](backport #36925) Fix event mapping implementation for statsd module #36986

Merged

ritalwar pushed a commit that referenced this pull request Oct 30, 2023

Fix event mapping implementation for statsd module (#36925) (#36986)

1e80ba2

* Fix eventmapping implementation for statsd module.

ritalwar mentioned this pull request Nov 28, 2023

[O11y][Airflow] Add system tests for the integration package elastic/integrations#8564

Merged

4 tasks

Scholar-Li pushed a commit to Scholar-Li/beats that referenced this pull request Feb 5, 2024

Fix event mapping implementation for statsd module (elastic#36925)

ad1e04d

* Fix eventmapping implementation for statsd module.

This was referenced Apr 15, 2024

[statsd_input] Test failure - observed hit count 6 did not match expected hit count 3 elastic/integrations#9584

Closed

[O11y][StatsD Input] Update to Kibana 8.11 for beats fixes elastic/integrations#9596

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix event mapping implementation for statsd module #36925

Fix event mapping implementation for statsd module #36925

ritalwar commented Oct 21, 2023 •

edited by shmsr

Loading

mergify bot commented Oct 21, 2023

elasticmachine commented Oct 21, 2023 •

edited by jenkins-beats-ci bot

Loading

Build stats

lalit-satapathy commented Oct 23, 2023

ritalwar commented Oct 24, 2023

ritalwar commented Oct 24, 2023 •

edited

Loading

ritalwar commented Oct 24, 2023

ali786XI commented Oct 24, 2023

devamanv commented Oct 25, 2023

agithomas commented Oct 27, 2023

agithomas left a comment

ritalwar commented Oct 27, 2023

ali786XI commented Oct 27, 2023 •

edited

Loading

ali786XI commented Oct 27, 2023 •

edited

Loading

tommyers-elastic Oct 27, 2023

ritalwar Oct 30, 2023

tommyers-elastic Oct 27, 2023

ritalwar Oct 30, 2023

tommyers-elastic Oct 27, 2023

ritalwar Oct 30, 2023

Fix event mapping implementation for statsd module #36925

Fix event mapping implementation for statsd module #36925

Conversation

ritalwar commented Oct 21, 2023 • edited by shmsr Loading

Proposed commit message

Checklist

How to test this PR locally

Related issues

Screenshots

mergify bot commented Oct 21, 2023

elasticmachine commented Oct 21, 2023 • edited by jenkins-beats-ci bot Loading

💚 Build Succeeded

Build stats

❕ Flaky test report

🤖 GitHub comments

lalit-satapathy commented Oct 23, 2023

ritalwar commented Oct 24, 2023

ritalwar commented Oct 24, 2023 • edited Loading

ritalwar commented Oct 24, 2023

ali786XI commented Oct 24, 2023

Testing Details

Test Setup

Test Case Scenarios

devamanv commented Oct 25, 2023

agithomas commented Oct 27, 2023

agithomas left a comment

Choose a reason for hiding this comment

ritalwar commented Oct 27, 2023

ali786XI commented Oct 27, 2023 • edited Loading

ali786XI commented Oct 27, 2023 • edited Loading

tommyers-elastic Oct 27, 2023

Choose a reason for hiding this comment

ritalwar Oct 30, 2023

Choose a reason for hiding this comment

tommyers-elastic Oct 27, 2023

Choose a reason for hiding this comment

ritalwar Oct 30, 2023

Choose a reason for hiding this comment

tommyers-elastic Oct 27, 2023

Choose a reason for hiding this comment

ritalwar Oct 30, 2023

Choose a reason for hiding this comment

ritalwar commented Oct 21, 2023 •

edited by shmsr

Loading

elasticmachine commented Oct 21, 2023 •

edited by jenkins-beats-ci bot

Loading

ritalwar commented Oct 24, 2023 •

edited

Loading

ali786XI commented Oct 27, 2023 •

edited

Loading

ali786XI commented Oct 27, 2023 •

edited

Loading