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

intake: add cloud.service.name to metadata #4626

Merged
merged 6 commits into from
Jan 25, 2021

Conversation

simitt
Copy link
Contributor

@simitt simitt commented Jan 15, 2021

Motivation/summary

This PR adds cloud.service.name to the set of available cloud information in the metadata. The field was introduced with elastic/ecs#1204.

Checklist

For functional changes, consider:

  • Is it observable through the addition of either logging or metrics?
  • Is its use being published in telemetry to enable product improvement?
  • Have system tests been added to avoid regression?

How to test these changes

Send a metadata.cloud.service.name information along with event data and check that it is indexed as keyword for error,transaction,span,metric events.

Related issues

closes 4625

How to add a new Intake API Field and index it

@simitt simitt added the v7.12.0 label Jan 15, 2021
@apmmachine
Copy link
Contributor

apmmachine commented Jan 15, 2021

💚 Build Succeeded

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

Expand to view the summary

Build stats

  • Build Cause: Pull request #4626 updated

    • Start Time: 2021-01-25T13:52:18.229+0000
  • Duration: 32 min 35 sec

  • Commit: 92f6dc3

Test stats 🧪

Test Results
Failed 0
Passed 4358
Skipped 126
Total 4484

Steps errors 3

Expand to view the steps failures

Compress
  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=coverage-files.tgz -czf coverage-files.tgz coverage
Compress
  • Took 0 min 0 sec . View more details on here
  • Description: tar --exclude=system-tests-linux-files.tgz -czf system-tests-linux-files.tgz system-tests
Test Sync
  • Took 4 min 43 sec . View more details on here
  • Description: ./.ci/scripts/sync.sh

Copy link
Contributor

@jalvz jalvz left a comment

Choose a reason for hiding this comment

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

Maybe add the field to docs/data/elasticsearch/generated?

Also, it would be great to create some section in the README with a link to this PR as a reference implementation of how to add a field to the intake (things have changed...)

@@ -96,6 +96,7 @@ Traces are written to `traces-apm.*` indices.
|cloud.project.name|Cloud project name|keyword| ![](https://doc-icons.s3.us-east-2.amazonaws.com/icon-yes.png) |
|cloud.provider|Cloud provider name|keyword| ![](https://doc-icons.s3.us-east-2.amazonaws.com/icon-yes.png) |
|cloud.region|Cloud region name|keyword| ![](https://doc-icons.s3.us-east-2.amazonaws.com/icon-yes.png) |
|cloud.service.name|Cloud service name, intended to distinguish services running on different platforms within a provider.|keyword| ![](https://doc-icons.s3.us-east-2.amazonaws.com/icon-no.png) |
Copy link
Contributor

Choose a reason for hiding this comment

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

this is wrong, isn't it? I think we need to update the ECS version

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It hasn't been released, so for now this is true.

simitt and others added 3 commits January 18, 2021 16:41
@simitt
Copy link
Contributor Author

simitt commented Jan 18, 2021

@jalvz good idea; I updated the description to contain a How to add a new Intake API Field and index it section and linked to it from the updated Contribution guid.

@simitt
Copy link
Contributor Author

simitt commented Jan 25, 2021

When running go test -race -v ./... the TestStorageGC still fails with these changes. I don't think the PR introduces an actual bug but rather that it reveals a test issue. @axw to unblock this PR, are you ok with skipping the test for now and creating an issue to rework it?

@axw
Copy link
Member

axw commented Jan 25, 2021

@simitt yes, that's fine - thanks and sorry.

@simitt simitt merged commit b456993 into elastic:master Jan 25, 2021
axw pushed a commit to axw/apm-server that referenced this pull request Feb 18, 2021
closes elastic#4625

# Conflicts:
#	changelogs/head.asciidoc
#	include/fields.go
axw added a commit that referenced this pull request Feb 18, 2021
closes #4625

# Conflicts:
#	changelogs/head.asciidoc
#	include/fields.go

Co-authored-by: Silvia Mitter <silvia.mitter@elastic.co>
@axw axw added the test-plan label Feb 20, 2021
@jalvz
Copy link
Contributor

jalvz commented Feb 23, 2021

The go agent doesn't let you trick into thinking is running on cloud, so I just sent handcrafted payloads to apm-server. Data is ingested as expected

@jalvz jalvz self-assigned this Feb 23, 2021
@simitt simitt deleted the cloud-service-name branch August 20, 2021 07:34
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants