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 real tests for splunk logger using vector #9526

Merged
merged 10 commits into from
Jun 2, 2023

Conversation

Revolyssup
Copy link
Contributor

@Revolyssup Revolyssup commented May 23, 2023

Description

Fixes #9489
This PR replaces mock splunk-hec-logger tests with real end to end test using vector as log server.

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)

Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
@monkeyDluffy6017
Copy link
Contributor

@Sn0rt please help to review

["hello world\n", "hello world\n", "hello world\n"]
--- no_error_log
[error]
qr/.*test batched data.*/
Copy link
Contributor

Choose a reason for hiding this comment

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

new line

@@ -187,18 +187,6 @@ services:
networks:
opa_net:

# Splunk HEC Logging Service
splunk:
Copy link
Contributor

Choose a reason for hiding this comment

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

why remove this component?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because we don't need a separate splunk server now. Vector does the job

Copy link
Contributor

Choose a reason for hiding this comment

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

ok

@monkeyDluffy6017
Copy link
Contributor

please resolve conflicts

Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
@@ -406,19 +400,23 @@ the mock backend is hit
if code >= 300 then
ngx.status = code
end
ngx.say(body)
local code, _, body2 = t("/hello", "GET")
Copy link
Contributor

Choose a reason for hiding this comment

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

need a blank

ngx.status = code
ngx.say(body)
return
end
Copy link
Contributor

Choose a reason for hiding this comment

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

need a blank

if code >= 300 then
ngx.status = code
ngx.say(body)
return
end

local code, _, body2 = t("/hello", "GET")
Copy link
Contributor

Choose a reason for hiding this comment

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

need a blank

ngx.say(body)
return
end
local code, body = t('/apisix/admin/routes/1', ngx.HTTP_PUT, {
Copy link
Contributor

Choose a reason for hiding this comment

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

need a blank

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added newline at all suggested place @monkeyDluffy6017

@monkeyDluffy6017 monkeyDluffy6017 added the wait for update wait for the author's response in this issue/PR label May 29, 2023
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
Signed-off-by: revolyssup <ashishjaitiwari15112000@gmail.com>
@monkeyDluffy6017
Copy link
Contributor

Could you merge the master? The ci always failed because the go packages faile to download.

@Revolyssup
Copy link
Contributor Author

Could you merge the master? The ci always failed because the go packages faile to download.

@monkeyDluffy6017 I merged master. Please review

@monkeyDluffy6017 monkeyDluffy6017 added approved and removed wait for update wait for the author's response in this issue/PR labels Jun 2, 2023
@monkeyDluffy6017 monkeyDluffy6017 merged commit 6ba4f73 into apache:master Jun 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat: As a user, I want the test of HEC logger with some real cases, so that
4 participants