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

Add tags field to internal_metrics data stream #8292

Merged
merged 1 commit into from Jun 3, 2022

Conversation

axw
Copy link
Member

@axw axw commented Jun 3, 2022

Motivation/summary

Documents added to metrics-apm.internal-<namespace> may have client.ip, and may require geoIP enrichment. If the geoip ingest processor cannot download the database, it will add a tag to the document. As we do not have the tags field defined, and we use strict mapping for this data stream, this leads to an indexing error.

Checklist

- [ ] Update CHANGELOG.asciidoc

  • Update package changelog.yml (only if changes to apmpackage have been made)
    - [ ] Documentation has been updated

How to test these changes

  1. Modify docker-compose.yml's Elasticsearch service, adding ingest.geoip.downloader.enabled=false to its environment and removing the ingest-geoip volume.
  2. Send some metrics to the RUM events endpoint with a public IP set. e.g. curl -H "X-Real-Ip: 8.8.8.8" -X POST -H Content-Type:application/x-ndjson http://localhost:8200/intake/v2/rum/events --data-binary @testdata/intake-v2/metricsets.ndjson

With the fix, metric documents should be indexed with a tags field.

Related issues

Closes #7465

@axw axw added v8.3.0 backport-8.3 Automated backport with mergify backport-8.2 Automated backport with mergify labels Jun 3, 2022
@axw axw marked this pull request as ready for review June 3, 2022 02:54
@axw axw requested a review from a team June 3, 2022 02:54
@apmmachine
Copy link
Collaborator

apmmachine commented Jun 3, 2022

💔 Tests Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-06-03T02:53:03.948+0000

  • Duration: 29 min 7 sec

Test stats 🧪

Test Results
Failed 1
Passed 4008
Skipped 13
Total 4022

Test errors 1

Expand to view the tests failures

Build and Test / System and Environment Tests / TestAggregate – github.com/elastic/apm-server/systemtest/benchtest/expvar
    Expand to view the error details

     Failed 
    

    Expand to view the stacktrace

     === RUN   TestAggregate
        metrics_test.go:64: 
            	Error Trace:	metrics_test.go:64
            	Error:      	"48" is not greater than or equal to "72"
            	Test:       	TestAggregate
    --- FAIL: TestAggregate (0.06s)
     
    

Steps errors 1

Expand to view the steps failures

Run Linux tests
  • Took 16 min 52 sec . View more details here
  • Description: ./.ci/scripts/linux-test.sh

🤖 GitHub comments

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

  • /test : Re-trigger the build.

  • /hey-apm : Run the hey-apm benchmark.

  • /package : Generate and publish the docker images.

  • /test windows : Build & tests on Windows.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@apmmachine
Copy link
Collaborator

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (42/42) 💚
Files 91.96% (183/199) 👍
Classes 93.421% (426/456) 👍
Methods 89.265% (1081/1211) 👍 0.083
Lines 76.818% (13162/17134) 👍 0.035
Conditionals 100.0% (0/0) 💚

@axw axw merged commit 6c84207 into elastic:main Jun 3, 2022
@axw axw deleted the internal-metrics-tags branch June 3, 2022 03:53
mergify bot pushed a commit that referenced this pull request Jun 3, 2022
mergify bot pushed a commit that referenced this pull request Jun 3, 2022
(cherry picked from commit 6c84207)

# Conflicts:
#	apmpackage/apm/changelog.yml
axw added a commit that referenced this pull request Jun 3, 2022
…#8297)

* Add tags field to internal_metrics data stream (#8292)

(cherry picked from commit 6c84207)

# Conflicts:
#	apmpackage/apm/changelog.yml

* Fix merge conflict

Co-authored-by: Andrew Wilkins <axw@elastic.co>
axw added a commit that referenced this pull request Jun 3, 2022
(cherry picked from commit 6c84207)

Co-authored-by: Andrew Wilkins <axw@elastic.co>
dedemorton added a commit to bmorelli25/apm-server that referenced this pull request Jun 9, 2022
dedemorton added a commit that referenced this pull request Jun 13, 2022
* docs: add 8.2.3 release notes

* Add fix #8292 to the release notes

* Update changelogs/8.2.asciidoc

Co-authored-by: DeDe Morton <dede.morton@elastic.co>
mergify bot pushed a commit that referenced this pull request Jun 13, 2022
* docs: add 8.2.3 release notes

* Add fix #8292 to the release notes

* Update changelogs/8.2.asciidoc

Co-authored-by: DeDe Morton <dede.morton@elastic.co>
(cherry picked from commit 6b4727a)
mergify bot pushed a commit that referenced this pull request Jun 13, 2022
* docs: add 8.2.3 release notes

* Add fix #8292 to the release notes

* Update changelogs/8.2.asciidoc

Co-authored-by: DeDe Morton <dede.morton@elastic.co>
(cherry picked from commit 6b4727a)
dedemorton added a commit that referenced this pull request Jun 14, 2022
* docs: add 8.2.3 release notes (#8313)

* docs: add 8.2.3 release notes

* Add fix #8292 to the release notes

* Update changelogs/8.2.asciidoc

Co-authored-by: DeDe Morton <dede.morton@elastic.co>
(cherry picked from commit 6b4727a)

* fix link

Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
Co-authored-by: DeDe Morton <dede.morton@elastic.co>
dedemorton added a commit that referenced this pull request Jun 14, 2022
* docs: add 8.2.3 release notes (#8313)

* docs: add 8.2.3 release notes

* Add fix #8292 to the release notes

* Update changelogs/8.2.asciidoc

Co-authored-by: DeDe Morton <dede.morton@elastic.co>
(cherry picked from commit 6b4727a)

* fix link

Co-authored-by: Brandon Morelli <brandon.morelli@elastic.co>
Co-authored-by: DeDe Morton <dede.morton@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-8.2 Automated backport with mergify backport-8.3 Automated backport with mergify v8.3.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

GeoIP processor failures will block data ingestion
3 participants