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.

Sign up for GitHub

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

Jump to bottom

ci: tcp logger with real logger server #9392

Merged
merged 4 commits into from
May 6, 2023
Merged

ci: tcp logger with real logger server #9392

merged 4 commits into from
May 6, 2023

Conversation

Sn0rt
Copy link
Contributor

@Sn0rt Sn0rt commented Apr 28, 2023 •
edited
Loading

Description

#9391

the vector is a popular logger service implemented by Rust

to add the vector as the logger server for receiving the log from the user client.

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)

@Sn0rt
ci: tcp logger with real logger server
9001580 
Signed-off-by: sn0rt <wangguohao.2009@gmail.com>
bzp2010
bzp2010 reviewed Apr 28, 2023
View reviewed changes
Copy link
Contributor

@bzp2010 bzp2010 left a comment

Choose a reason for hiding this comment

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

For the content, I see no problem. But I have a question: is there any reason why you chose vector?

@Sn0rt
Copy link
Contributor Author

Sn0rt commented Apr 28, 2023 •
edited
Loading

For the content, I see no problem. But I have a question: is there any reason why you chose vector?

Simple, easy to configure, well documented.

There is another reason why I tried to use Fluentd and encountered a strange case. I spent half a day trying to fix it to no avail.

@bzp2010
Copy link
Contributor

bzp2010 commented Apr 28, 2023 •
edited
Loading

Simple, easy to configure, well documented.

There is another reason why I tried to use Fluentd and encountered a strange case. I spent half a day trying to fix it to no avail.

OK, @Sn0rt

For the first point, that's very good. And for the second point, have you tried fluent bit (it is the next generation of fluentd)?

@Sn0rt
Copy link
Contributor Author

Sn0rt commented Apr 28, 2023 •
edited
Loading

Simple, easy to configure, well documented.
There is another reason why I tried to use Fluentd and encountered a strange case. I spent half a day trying to fix it to no avail.

OK, @Sn0rt

For the first point, that's very good. And for the second point, have you tried fluent bit (it is the next generation of fluentd)?

I took a brief look at the document construction of fluent-bit, which is not as good as that of vector. My requirement is to cooperate with multiple reporting log formations. fluent-bit does not have an example available out of the box, but vector provides it.

In this PR, I expect the vector to become the basic component for the CI log plugins test, and to cooperate with other log plugins later.

@bzp2010
Copy link
Contributor

bzp2010 commented Apr 28, 2023 •
edited
Loading

@Sn0rt Understand and recognize your position.

But I'm not sure if it's the right strategy to keep using vector in the future because essentially the open source project is a vendor-neutral project, the plugin is called tcp-logger and users may use any logging pipeline project that supports TCP, I'm not sure....

Maybe consider adding testing via other log pipeline projects in a future PR. Approved this PR now.

bzp2010
bzp2010 previously approved these changes Apr 28, 2023
View reviewed changes
shreemaan-abhishek
shreemaan-abhishek reviewed Apr 28, 2023
View reviewed changes
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

Neilblaze
Neilblaze previously approved these changes Apr 28, 2023
View reviewed changes
soulbird
soulbird reviewed Apr 30, 2023
View reviewed changes
ci/pod/vector/vector_logs_ca.key Outdated
@@ -0,0 +1,27 @@
-----BEGIN RSA PRIVATE KEY-----
Copy link
Contributor

Choose a reason for hiding this comment

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

We'd better move certs to t/certs

Copy link
Contributor Author

Choose a reason for hiding this comment

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

updated

@Sn0rt
ci: move the certificates to /t/certs/
25b16e2 
Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
@Sn0rt Sn0rt dismissed stale reviews from Neilblaze and bzp2010 via 25b16e2 May 4, 2023 01:54
@Sn0rt
ci: tcp-logger.t change the order of the tests
983b03f 
Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
@soulbird
Copy link
Contributor

soulbird commented May 5, 2023

We need to adjust the existing test cases. Currently, the log is forwarded to port 5044, which is a simulated log service.

@Sn0rt
Copy link
Contributor Author

Sn0rt commented May 5, 2023

We need to adjust the existing test cases. Currently, the log is forwarded to port 5044, which is a simulated log service.

copy that. I rewrite some of the tcp-logger test cases.

@Sn0rt
ci: tcp-logger remove mock test case
0dcded2 
Signed-off-by: Sn0rt <wangguohao.2009@gmail.com>
@monkeyDluffy6017 monkeyDluffy6017 merged commit 02ee09e into apache:master May 6, 2023
hongbinhsu added a commit to fitphp/apix that referenced this pull request May 13, 2023
@hongbinhsu
Merge branch 'upstream/master' into github/master
416f768 
* upstream/master: (42 commits)
  fix: add `rust-check` target to the makefile for deps pre-checking (apache#9453)
  test: remove duplicate `proto` unit test file and tweak test case title (apache#9445)
  feat: proxy-mirror add grpc and grpcs support (apache#9388)
  test: replace all httpbin upstream (apache#9452)
  fix: 404 error in ci test (apache#9447)
  fix: body-transformer plugin return raw body anytime (apache#9446)
  fix(ci): fips: ensure apisix compiles with openssl3 (apache#9427)
  chore(deps): bump actions/stale from 7 to 8 (apache#9159)
  chore(deps): bump actions/setup-node from 3.5.1 to 3.6.0 (apache#8625)
  chore(test): split the access log of apisix and test server (apache#8819)
  docs: fix grammar (apache#9395)
  fix(wolf-rbac): other plugin in consumer not effective when consumer used wolf-rbac plugin (apache#9287) (apache#9298)
  docs: use shell instead of Python to configure protos resources (apache#9414)
  docs: add guide to authenticate websocket connections (apache#9369)
  docs: updated Stream Proxy doc (apache#9367)
  feat: proxy_rewrite support miltiple regex pattern matching (apache#9194)
  feat: route-level MTLS (apache#9322)
  ci: tcp logger with real logger server (apache#9392)
  fix: always parse domain when host is domain name (apache#9332)
  docs: CHANGELOG.md Chinese docs lint (apache#9411)
  ...
AlinsRan pushed a commit to AlinsRan/apisix that referenced this pull request Jun 26, 2023
@Sn0rt @AlinsRan
ci: tcp logger with real logger server (apache#9392)
33de9a8
@Sn0rt Sn0rt deleted the sn0rt/new-ci-4-tcp-logger branch September 20, 2023 02:39
@Sn0rt Sn0rt restored the sn0rt/new-ci-4-tcp-logger branch September 20, 2023 02:39
@Sn0rt Sn0rt deleted the sn0rt/new-ci-4-tcp-logger branch September 20, 2023 02:39
@Sn0rt Sn0rt restored the sn0rt/new-ci-4-tcp-logger branch September 20, 2023 02:39
@Sn0rt Sn0rt deleted the sn0rt/new-ci-4-tcp-logger branch September 20, 2023 02:39
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@monkeyDluffy6017 monkeyDluffy6017 monkeyDluffy6017 approved these changes

@soulbird soulbird soulbird approved these changes

@shreemaan-abhishek shreemaan-abhishek Awaiting requested review from shreemaan-abhishek

@bzp2010 bzp2010 Awaiting requested review from bzp2010

@Neilblaze Neilblaze Awaiting requested review from Neilblaze

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@Sn0rt @bzp2010 @soulbird @monkeyDluffy6017 @Neilblaze @shreemaan-abhishek