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 elasticsearch_capture_body_urls option #670

Merged
merged 6 commits into from
Aug 18, 2022

Conversation

felixbarny
Copy link
Member

@felixbarny felixbarny commented Aug 10, 2022

Refs: elastic/apm-agent-nodejs#2019
Closes: #665

We're only expecting to implement this in the Node.js agent in the near future.
Other agents may add this to their roadmap as they see fit but we won't create a cross-agent tracking issue.

  • May the instrumentation collect sensitive information, such as secrets or PII (ex. in headers)?
    • Yes
      • Add a section to the spec how agents should apply sanitization (such as sanitize_field_names) We're only capturing the request body for search requests by default to minimize the risk. Users can also add a custom ingest node pipeline to parse and redact JSON if needed.
    • No
      • Why?
    • n/a
  • Create PR as draft
  • Approval by at least one other agent
  • Mark as Ready for Review (automatically requests reviews from all agents and PM via CODEOWNERS)
    • Remove PM from reviewers if impact on product is negligible
    • Remove agents from reviewers if the change is not relevant for them
  • Approved by at least 2 agents + PM (if relevant)
  • Merge after 7 days passed without objections
  • Create implementation issues through the meta issue template (this will automate issue creation for individual agents)

/schedule 2022-08-18

@felixbarny felixbarny requested review from a team as code owners August 10, 2022 06:17
@apmmachine
Copy link

apmmachine commented Aug 10, 2022

💚 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: 2022-08-16T11:14:28.625+0000

  • Duration: 3 min 55 sec

Copy link
Member

@danielmitterdorfer danielmitterdorfer left a comment

Choose a reason for hiding this comment

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

Thanks for quick update! LGTM

Copy link
Member

@trentm trentm left a comment

Choose a reason for hiding this comment

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

Thanks for starting this Felix!

specs/agents/tracing-instrumentation-db.md Outdated Show resolved Hide resolved
Co-authored-by: Trent Mick <[email protected]>
@trentm
Copy link
Member

trentm commented Aug 12, 2022

Closes elastic/apm-agent-nodejs#2019 and #665

@felixbarny Are you okay with me changing that "Closes" to "Refs", so that the former issue isn't actually closed when this spec goes in? The issue should be closed when we implement the spec.

Update:

Oh, actually, I think what's wanted is:

Refs: https://github.com/elastic/apm-agent-nodejs/issues/2019
Closes: #665

@felixbarny
Copy link
Member Author

As this affects all agents at least in the way the default behaves, I have created implementation issues for all agents and I'm waiting two more days for objections.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
6 participants