Drop observer.id and observer.ephemeral_id
#9412
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
This pull request does not have a backport label. Could you fix it @axw? 🙏
NOTE: |
mergify
bot
added
the
backport-skip
axw
force-pushed
the
beater-no-beatinfo-part2
branch
3 times, most recently
from
5c68b7c to
5968c29
Compare
💚 Build Succeeded
Expand to view the summary
Build stats
Test stats 🧪
🤖 GitHub commentsExpand to view the GitHub comments
To re-run your PR in the CI, just comment with:
|
📚 Go benchmark reportDiff with the report generated with https://pkg.go.dev/golang.org/x/perf/cmd/benchstat |
Package beater now takes just the UUIDs for `observer.id` and `observer.ephemeral_id`. The `observer.type` field is hard-coded, `observer.version` comes from package version, and `observer.hostname` comes from a call to `os.Hostname()`. We need to recreate `beat.Info` for the libbeat outputs. If at some point we can get rid of those, then we can remove this.
axw
force-pushed
the
beater-no-beatinfo-part2
branch
2 times, most recently
from
6f98025 to
44f3bef
Compare
We don't use these fields in the UI anywhere, and for debugging we can rely on the hostname to distinguish one APM Server instance from another. The observer.id and observer.ephemeral_id fields suggest that we should care about specific instances, and their continued presence contradicts our intentions for a more modern "cloud native" approach, where APM Server instances may come and go. For now we need to continue persisting a UUID to disk for stack monitoring, as this is used to identify instances of APM Server. In the future we expect to revisit how stack monitoring works, and at that point we should redesign the documents to not enshrine a "pets" mentality. For tail-based sampling we now generate a UUID (for identifying its documents) at process start. The field has been renamed from observer.id to agent.ephemeral_id, the latter of which is an ECS field (observer.id is not).
axw
force-pushed
the
beater-no-beatinfo-part2
branch
from
44f3bef to
f01864e
Compare
|
This pull request is now in conflicts. Could you fix it @axw? 🙏 |
simitt
reviewed
Oct 28, 2022
| processors: | ||
| - rename: | ||
| field: observer.id | ||
| target_field: agent.ephemeral_id |
There was a problem hiding this comment.
ECS says:
This id normally changes across restarts, but
agent.iddoes not.
I believe semantically observer.id -> agent.id would be more correct.
There was a problem hiding this comment.
The ID changes on restart now - it's generated at process init.
simitt
approved these changes
Oct 28, 2022
|
Tested on with BC5 on ESS that ingestion still works, including having TBS enabled across multiple APM Servers and also ensured Stack Monitoring is still working. |
|
Package apm - 8.6.0-preview-1670294014 containing this change is available at https://epr.elastic.co/search?package=apm |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Motivation/summary
We don't use these fields in the UI anywhere, and for debugging we can rely on the hostname to distinguish one APM Server instance from another. The
observer.idandobserver.ephemeral_idfields suggest that we should care about specific instances (and think of them as long-lived), and their continued presence contradicts our intentions for a more modern, cloud native, approach, where APM Server instances may come and go.For now we need to continue persisting a UUID to disk for stack monitoring, as this is used to identify instances of APM Server. In the future we expect to revisit how stack monitoring works, and at that point we should redesign the documents to not enshrine a "pets" mentality. We may still want to identify individual instances (e.g. for calculating time-series IDs), but statefulness should not be required.
For tail-based sampling we now generate a UUID (for identifying its documents) at process start. The field has been renamed from
observer.idtoagent.ephemeral_id, the latter of which is an ECS field (observer.idis not).We no longer pass
beat.Infofrombeatcmdinto packagebeater. Instead,beaterhard-codesobserver.type: apm-server, uses packageversionforobserver.version, and callsos.Hostname()forobserver.hostname. We need to recreatebeat.Infofor the libbeat outputs. If at some point we drop support for libbeat outputs, then we would have no more dependencies onbeat.Infooutside of packagebeatcmd.Checklist
apmpackagehave been made)How to test these changes
Related issues
Closes #3874