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

Introduce model.Processor #5984

Merged
merged 11 commits into from
Aug 24, 2021
Merged

Introduce model.Processor #5984

merged 11 commits into from
Aug 24, 2021

Conversation

axw
Copy link
Member

@axw axw commented Aug 20, 2021

Motivation/summary

Introduce type model.Processor and field model.APMEvent.Processor, which controls the processor.name and processor.event fields. These fields are used for identifying the event type, used for routing to the appropriate index/data stream and also for monitoring purposes.

Processed event metrics have been moved to a dedicated model.BatchProcessor, and out of the model package; they use the Processor field to form the metric name.

Note: Several unused metrics have been removed: stack trace and stack frame counts.

Checklist

How to test these changes

Enable monitoring, ingest some events, check that apm-server.processor.<event-type>.transformations metrics are recorded.

Related issues

#4410
#4120
#4715

@apmmachine
Copy link
Contributor

apmmachine commented Aug 20, 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 preview

Expand to view the summary

Build stats

  • Start Time: 2021-08-24T02:50:06.266+0000

  • Duration: 42 min 5 sec

  • Commit: 876955f

Test stats 🧪

Test Results
Failed 0
Passed 5940
Skipped 14
Total 5954

Trends 🧪

Image of Build Times

Image of Tests

axw added 5 commits August 21, 2021 10:35
Processor identifies the event kind: transaction, span,
metricset, error, or profile. This is used for routing
events to the appropriate index or data stream, and in
queries for fetching events of a given kind.
@axw axw force-pushed the model-event-processor branch from b09a075 to 9a20129 Compare August 21, 2021 02:41
@axw axw requested a review from a team August 23, 2021 03:56
@axw axw marked this pull request as ready for review August 23, 2021 03:56
@stuartnelson3 stuartnelson3 self-assigned this Aug 23, 2021
Copy link
Contributor

@stuartnelson3 stuartnelson3 left a comment

Choose a reason for hiding this comment

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

Big fan of the move to explicitly setting the datastream based on Processor. One question where it looks like processor might be getting double set, but that's all.

*batch = append(*batch, metricset)
event := input.Base
mapToMetricsetModel(&m, &event)
event.Processor = model.MetricsetProcessor
Copy link
Contributor

Choose a reason for hiding this comment

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

It appears this event.Processor = model.MetricsetProcessor is also being set in mapToMetricsetModel? Is that intentional?

Copy link
Member Author

Choose a reason for hiding this comment

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

good catch, I've removed these ones and consolidated on setting it in mapToFooModel

switch {
case event.Transaction != nil || event.Span != nil:
switch event.Processor {
case model.SpanProcessor, model.TransactionProcessor:
Copy link
Contributor

Choose a reason for hiding this comment

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

🥳

@stuartnelson3 stuartnelson3 removed their assignment Aug 23, 2021
@mergify
Copy link
Contributor

mergify bot commented Aug 24, 2021

This pull request is now in conflicts. Could you fix it @axw? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b model-event-processor upstream/model-event-processor
git merge upstream/master
git push upstream model-event-processor

@axw axw enabled auto-merge (squash) August 24, 2021 02:49
@axw axw merged commit cb6b2da into elastic:master Aug 24, 2021
mergify bot pushed a commit that referenced this pull request Aug 24, 2021
* Introduce model.Processor

Processor identifies the event kind: transaction, span,
metricset, error, or profile. This is used for routing
events to the appropriate index or data stream, and in
queries for fetching events of a given kind.

* Update SetDataStream to use APMEvent.Processor

* Move monitoring event counters to model processor

Also, remove unused metrics.

* Update aggregators to use Processor

* sampling: use Processor

* Update changelog

* Remove redundant setting of event.Processor

Co-authored-by: stuart nelson <[email protected]>
(cherry picked from commit cb6b2da)

# Conflicts:
#	changelogs/head.asciidoc
#	model/apmevent.go
#	model/apmevent_test.go
#	model/modeldecoder/rumv3/decoder.go
#	model/span.go
#	model/transaction.go
#	processor/otel/exceptions_test.go
#	x-pack/apm-server/sampling/groups_test.go
#	x-pack/apm-server/sampling/processor.go
#	x-pack/apm-server/sampling/processor_test.go
axw added a commit to axw/apm-server that referenced this pull request Aug 24, 2021
* Introduce model.Processor

Processor identifies the event kind: transaction, span,
metricset, error, or profile. This is used for routing
events to the appropriate index or data stream, and in
queries for fetching events of a given kind.

* Update SetDataStream to use APMEvent.Processor

* Move monitoring event counters to model processor

Also, remove unused metrics.

* Update aggregators to use Processor

* sampling: use Processor

* Update changelog

* Remove redundant setting of event.Processor

Co-authored-by: stuart nelson <[email protected]>
# Conflicts:
#	changelogs/head.asciidoc
axw added a commit that referenced this pull request Aug 24, 2021
* Introduce model.Processor

Processor identifies the event kind: transaction, span,
metricset, error, or profile. This is used for routing
events to the appropriate index or data stream, and in
queries for fetching events of a given kind.

* Update SetDataStream to use APMEvent.Processor

* Move monitoring event counters to model processor

Also, remove unused metrics.

* Update aggregators to use Processor

* sampling: use Processor

* Update changelog

* Remove redundant setting of event.Processor

Co-authored-by: stuart nelson <[email protected]>
# Conflicts:
#	changelogs/head.asciidoc
@marclop marclop added backport-skip Skip notification from the automated backport with mergify test-plan labels Oct 25, 2021
@marclop marclop self-assigned this Oct 28, 2021
@marclop
Copy link
Contributor

marclop commented Oct 28, 2021

Verified this running 7.16.0 BC1 locally:

  1. ./apm-server -E instrumentation.enabled=true -E output.elasticsearch.username=admin -E output.elasticsearch.password=changeme -E apm-server.expvar.enabled=true
  2. curl -sL http://localhost:8200/debug/vars | grep transformations:
"apm-server.processor.span.transformations": 58,
"apm-server.processor.transaction.transformations": 40,
"apm-server.processor.metric.transformations": 59,

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-skip Skip notification from the automated backport with mergify test-plan test-plan-ok v7.16.0
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants