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(file-logger): support standard output for file logger plugins #8681

Closed
wants to merge 11 commits into from

Conversation

e1ijah1
Copy link

@e1ijah1 e1ijah1 commented Jan 13, 2023

Description

support standard output for file logger plugins

Fixes #6797

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)

@e1ijah1 e1ijah1 marked this pull request as ready for review January 15, 2023 05:44
apisix/plugins/file-logger.lua Outdated Show resolved Hide resolved
apisix/plugins/file-logger.lua Outdated Show resolved Hide resolved
@e1ijah1 e1ijah1 requested a review from tokers January 16, 2023 03:00
apisix/plugins/file-logger.lua Outdated Show resolved Hide resolved
apisix/plugins/file-logger.lua Outdated Show resolved Hide resolved
Copy link
Member

@spacewander spacewander left a comment

Choose a reason for hiding this comment

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

@e1ijah1
Copy link
Author

e1ijah1 commented Jan 16, 2023

Can you add a test in https://github.com/apache/apisix/tree/master/t/cli?

I'm not quite sure how to capture the standard output in E2E testing. Could you please give me some hints if you're available? @spacewander

@e1ijah1 e1ijah1 requested review from spacewander and removed request for tokers January 16, 2023 09:58
@spacewander
Copy link
Member

Can you add a test in master/t/cli?

I'm not quite sure how to capture the standard output in E2E testing. Could you please give me some hints if you're available? @spacewander

You can add a custom configuration to let APISIX don't run as a daemon. See https://github.com/apache/apisix/blob/master/docs/en/latest/customize-nginx-configuration.md

apisix/plugins/file-logger.lua Show resolved Hide resolved
apisix/plugins/file-logger.lua Outdated Show resolved Hide resolved
docs/en/latest/plugins/file-logger.md Outdated Show resolved Hide resolved
@e1ijah1
Copy link
Author

e1ijah1 commented Jan 18, 2023

Can you add a test in https://github.com/apache/apisix/tree/master/t/cli?

The test has been added. Please take a look if you're available. @spacewander

apisix/plugins/file-logger.lua Outdated Show resolved Hide resolved
apisix/plugins/file-logger.lua Outdated Show resolved Hide resolved
t/cli/plugin/test_file_logger_std_output.sh Outdated Show resolved Hide resolved
t/cli/plugin/test_file_logger_std_output.sh Outdated Show resolved Hide resolved
@e1ijah1 e1ijah1 requested review from spacewander and tokers and removed request for spacewander and tokers January 28, 2023 12:43
@e1ijah1
Copy link
Author

e1ijah1 commented Jan 29, 2023

@spacewander It looks like the connection to the ETCD in one environment is down, causing CI to fail. Would you mind restarting the CI and reviewing my code?

apisix/cli/env.lua Outdated Show resolved Hide resolved
apisix/cli/util.lua Outdated Show resolved Hide resolved
t/cli/plugin/test_file_logger_std_output.sh Show resolved Hide resolved
chore: fmt code

chore: add test

chore: fmt code

chore: fmt code

chore: fix lint

chore: fmt code

chore: fmt code
chore: remove specify listen port in test config

chore: improve the code
fix lint

fix: add test path

fix ci

fix test

fix test
util.execute_cmd(env.openresty_args)
local data, err = util.execute_cmd(env.openresty_args)
if not err and data and data ~= "" then
print(data)
Copy link
Member

Choose a reason for hiding this comment

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

It is useless to print the access log only after the Nginx exited.

@@ -0,0 +1,82 @@
#!/usr/bin/env bash

Copy link
Member

Choose a reason for hiding this comment

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

Could we remove the intermediate directory, so this file can be tested without modifying the ci script?

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.

bug: cannot use file-logger to log to stdout
3 participants