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

feat: add option to include request body and response body in log util #10888

Merged
merged 13 commits into from Feb 2, 2024

Conversation

smileby
Copy link
Contributor

@smileby smileby commented Jan 30, 2024

Description

Fixes #10803

The request body and response body related attributes are added to the log plug-in. The list is as follows:

  • elasticsearch-logger
  • skywalking-logger
  • syslog
  • sls-logger
  • tcp-logger
  • udp-logger

upplement the description of existing attributes missing in the Chinese document. The list is as follows:

  • loggly
  • tencent-cloud-cls

Checklist

  • I have explained the need for this PR and the problem it solves
  • I have explained the changes or the new features added to this PR
  • I have added tests corresponding to this change
  • I have updated the documentation to reflect this change
  • I have verified that this change is backward compatible (If not, please discuss on the APISIX mailing list first)

@smileby
Copy link
Contributor Author

smileby commented Jan 31, 2024

@shreemaan-abhishek Please help to check, thanks

@smileby
Copy link
Contributor Author

smileby commented Jan 31, 2024

@shreemaan-abhishek I don’t think the ci error is caused by this PR, please help check. Thanks

Copy link
Contributor

@shreemaan-abhishek shreemaan-abhishek left a comment

Choose a reason for hiding this comment

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

please provide test cases for other plugnis as well.

@smileby
Copy link
Contributor Author

smileby commented Jan 31, 2024

please provide test cases for other plugnis as well.

@shreemaan-abhishek Can the test cases be completed by adding logs in the plug-in? I can't seem to use assertions without increasing the log

@shreemaan-abhishek
Copy link
Contributor

@smileby I didn't get you.

@monkeyDluffy6017
Copy link
Contributor

@smileby please resolve the conflicts

@smileby
Copy link
Contributor Author

smileby commented Feb 1, 2024

@smileby please resolve the conflicts

Thanks for the reminder, I have solved it

@smileby
Copy link
Contributor Author

smileby commented Feb 1, 2024

@smileby I didn't get you.

@shreemaan-abhishek I added the info log in the plug-in to facilitate me to write test cases. The code has been submitted, please help to check, thank you

Copy link
Contributor

@shreemaan-abhishek shreemaan-abhishek left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

@monkeyDluffy6017 monkeyDluffy6017 merged commit 07c4aa3 into apache:master Feb 2, 2024
47 checks passed
@smileby smileby deleted the feat_include-body branch February 2, 2024 14:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: Add schema attribute definition for some log plug-ins
3 participants