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

publish: implement BatchProcessor; remove tracing #6243

Merged
merged 3 commits into from
Oct 4, 2021

Conversation

axw
Copy link
Member

@axw axw commented Sep 26, 2021

Motivation/summary

  • Modify publish.Publisher so that it implements model.BatchProcessor directly.
  • Remove the tracing instrumentation from publisher. We don't use and don't support libbeat processors, so this is not useful.

Checklist

How to test these changes

  1. Enable instrumentation
  2. Send some events to apm-server
  3. Check that no "Publisher" type transactions are traced

Related issues

Related to #6002

@apmmachine
Copy link
Contributor

apmmachine commented Sep 26, 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-10-04T06:16:49.202+0000

  • Duration: 50 min 43 sec

  • Commit: 655e6a8

Test stats 🧪

Test Results
Failed 0
Passed 6141
Skipped 14
Total 6155

🤖 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.

Modify publish.Publisher so that it implements
model.BatchProcessor directly. Remove the tracing
instrumentation from publisher. We no longer use
and don't support libbeat processors, so this is
not useful.
@axw axw force-pushed the publish-modelprocessor branch from f9e0feb to 5cc94a9 Compare September 26, 2021 08:03
beater/beater.go Show resolved Hide resolved
@axw axw marked this pull request as ready for review October 4, 2021 05:22
@mergify
Copy link
Contributor

mergify bot commented Oct 4, 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 publish-modelprocessor upstream/publish-modelprocessor
git merge upstream/master
git push upstream publish-modelprocessor

@axw axw enabled auto-merge (squash) October 4, 2021 06:25
@axw axw merged commit 79b43c1 into elastic:master Oct 4, 2021
mergify bot pushed a commit that referenced this pull request Oct 4, 2021
Modify publish.Publisher so that it implements
model.BatchProcessor directly. Remove the tracing
instrumentation from publisher. We no longer use
and don't support libbeat processors, so this is
not useful.

(cherry picked from commit 79b43c1)

# Conflicts:
#	changelogs/head.asciidoc
axw added a commit that referenced this pull request Oct 4, 2021
…) (#6279)

* publish: implement BatchProcessor; remove tracing (#6243)

Modify publish.Publisher so that it implements
model.BatchProcessor directly. Remove the tracing
instrumentation from publisher. We no longer use
and don't support libbeat processors, so this is
not useful.

(cherry picked from commit 79b43c1)

# Conflicts:
#	changelogs/head.asciidoc

* Delete head.asciidoc

Co-authored-by: Andrew Wilkins <[email protected]>
@mergify
Copy link
Contributor

mergify bot commented Oct 25, 2021

This pull request does not have a backport label. Could you fix it @axw? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-7.x is the label to automatically backport to the 7.x branch.
  • backport-7./d is the label to automatically backport to the 7./d branch. /d is the digit

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Oct 25, 2021
@marclop marclop self-assigned this Nov 3, 2021
@marclop
Copy link
Contributor

marclop commented Nov 4, 2021

Tested with 7.16.0 BC2 locally:

  1. ./apm-server -E output.elasticsearch.username=admin -E output.elasticsearch.password=changeme -E instrumentation.enabled=true -e
  2. Verify that no results are returned with the following query:
GET apm-*,traces-apm*,logs-apm*/_search
{
  "query": {
    "bool": {
      "should": [
        {
          "match": {
            "transaction.type": "Publisher"
          }
        },
        {
          "match": {
            "span.type": "Publisher"
          }
        }
      ]
    }
  }
}

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.

3 participants