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 request tracer toggle for httpjson input type #5641

Closed
wants to merge 87 commits into from

Conversation

bhapas
Copy link
Contributor

@bhapas bhapas commented Mar 22, 2023

What does this PR do?

This PR sets a toggle to enable_request_tracing on all httpjson input integrations. It logs the raw requests and responses to a dedicated file. The allows users to collect detailed debugging data that is included in the agent diagnostics dump.

This also sets minimum kibana version to 8.7.1 as the changes to handle the tracer filename placeholder * are incorporated in this version.

Checklist

  • I have reviewed tips for building integrations and this pull request is aligned with them.
  • I have verified that all data streams collect metrics or logs.
  • I have added an entry to my package's changelog.yml file.
  • I have verified that Kibana version constraints are current according to guidelines.

How to test this PR locally

Related

@bhapas bhapas added the enhancement New feature or request label Mar 22, 2023
@bhapas bhapas self-assigned this Mar 22, 2023
@elasticmachine
Copy link

elasticmachine commented Mar 22, 2023

💔 Build Failed

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: 2023-04-19T12:07:39.935+0000

  • Duration: 67 min 27 sec

Test stats 🧪

Test Results
Failed 0
Passed 1482
Skipped 4
Total 1486

Steps errors 2

Expand to view the steps failures

Test integration: zeek
  • Took 46 min 1 sec . View more details here
  • Description: eval "$(../../build/elastic-package stack shellinit)" ../../build/elastic-package test -v --report-format xUnit --report-output file --test-coverage
Google Storage Download
  • Took 0 min 0 sec . View more details here

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

@elasticmachine
Copy link

elasticmachine commented Mar 22, 2023

🌐 Coverage report

Name Metrics % (covered/total) Diff
Packages 100.0% (165/165) 💚
Files 96.757% (179/185) 👎 -0.612
Classes 96.757% (179/185) 👎 -0.612
Methods 91.912% (2216/2411) 👍 0.51
Lines 92.169% (59297/64335) 👍 4.503
Conditionals 100.0% (0/0) 💚

@bhapas bhapas marked this pull request as ready for review April 19, 2023 12:21
@bhapas bhapas requested review from a team as code owners April 19, 2023 12:21
@bhapas bhapas requested review from rdner and cmacknz April 19, 2023 12:21
Copy link
Member

@andrewkroh andrewkroh left a comment

Choose a reason for hiding this comment

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

I think it would be good to enable the feature in system tests where they exist. Merely exercising the feature could expose critical issues if the feature ever were to trigger something major like a panic. And if someone is debugging system tests that trace log file will already exist for them to analyze.

@bhapas
Copy link
Contributor Author

bhapas commented May 9, 2023

Breaking this down into multiple PRs to accommodate easy review process

@bhapas
Copy link
Contributor Author

bhapas commented May 9, 2023

#6115 - For SEI related integrations

@ishleenk17
Copy link
Contributor

@bhapas : I see that plan is to segregate the files as per ownership.
Do we have the PR which is required to be reviewed by obs-service-integrations team ?

@bhapas
Copy link
Contributor Author

bhapas commented May 11, 2023

@bhapas : I see that plan is to segregate the files as per ownership. Do we have the PR which is required to be reviewed by obs-service-integrations team ?

@ishleenk17 Not yet... I am fixing the commits... I will put up a PR and link it here and ping your team

@bhapas
Copy link
Contributor Author

bhapas commented May 11, 2023

@bhapas : I see that plan is to segregate the files as per ownership. Do we have the PR which is required to be reviewed by obs-service-integrations team ?

@ishleenk17 Not yet... I am fixing the commits... I will put up a PR and link it here and ping your team

@ishleenk17 #6160 is the PR

@bhapas bhapas mentioned this pull request May 11, 2023
4 tasks
@bhapas
Copy link
Contributor Author

bhapas commented May 11, 2023

#6161 For obs-cloud-monitoring team

@bhapas
Copy link
Contributor Author

bhapas commented May 11, 2023

#6163 for System and Windows integrations

@shmsr
Copy link
Member

shmsr commented May 11, 2023

@bhapas Can you please resolve the merge conflicts and rebase against main branch? It'd be helpful when reviewing.

@bhapas
Copy link
Contributor Author

bhapas commented May 11, 2023

@bhapas Can you please resolve the merge conflicts and rebase against main branch? It'd be helpful when reviewing.

#6160 is the PR that is broken down from this huge PR.

@bhapas
Copy link
Contributor Author

bhapas commented May 11, 2023

#6115 #6160 #6161 #6163 are the broken down PRs.

Closing this big PR to avoid confusion

@bhapas bhapas closed this May 11, 2023
@bhapas bhapas deleted the agent-diag branch May 11, 2023 17:47
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.