apisix icon indicating copy to clipboard operation
apisix copied to clipboard

feat: support Tencent Cloud Log Service

Open ychensha opened this issue 2 years ago • 9 comments

Description

product doc

apisix log util is more powerfull than nginx access log, and it's useful for debugging and replaying later.

Fixes # (7592)

Checklist

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

ychensha avatar Aug 02 '22 08:08 ychensha

Thanks for your contribution. But please add a license to each file and add test cases first.

soulbird avatar Aug 03 '22 01:08 soulbird

any doc for testing? I have no exp for env setup & PERL testing framework...

ychensha avatar Aug 03 '22 02:08 ychensha

any doc for testing? I have no exp for env setup & PERL testing framework...

See: https://github.com/apache/apisix/blob/master/docs/en/latest/internal/testing-framework.md

soulbird avatar Aug 03 '22 03:08 soulbird

any doc for testing? I have no exp for env setup & PERL testing framework...

See: https://github.com/apache/apisix/blob/master/docs/en/latest/internal/testing-framework.md

tried.. I work on Mac, and now trapped in etcd configuration for more tesing case

ychensha avatar Aug 03 '22 11:08 ychensha

I have a question: Is it that we can't do the test case to verify that this plugin is working correctly? Because it requires a Tencent account and to make sure that this account is always available.

tzssangglass avatar Aug 10 '22 03:08 tzssangglass

I have a question: Is it that we can't do the test case to verify that this plugin is working correctly? Because it requires a Tencent account and to make sure that this account is always available.

We can inject a hook or mock into the code? Like: https://github.com/apache/apisix/blob/b3d4b464045b9bdfed3170cbd7a343ed5c40b24a/t/plugin/zipkin2.t#L35 https://github.com/apache/apisix/blob/b3d4b464045b9bdfed3170cbd7a343ed5c40b24a/t/plugin/opentelemetry.t#L553

spacewander avatar Aug 10 '22 06:08 spacewander

I have same question too. But same question can be applie to other plugins that have env matters

ychensha avatar Aug 10 '22 06:08 ychensha

I have same question too. But same question can be applie to other plugins that have env matters

ychensha avatar Aug 10 '22 06:08 ychensha

@spacewander @tzssangglass after code related conversations closed, I will do the function test again for the related code modification.

ychensha avatar Aug 10 '22 06:08 ychensha

We can inject a hook or mock into the code? Like:

@ychensha Let's do that for now, there's no better way to do it.

tzssangglass avatar Aug 10 '22 17:08 tzssangglass

If hook is ok for now, I will add test code like otel plugin.

ychensha avatar Aug 11 '22 05:08 ychensha

If hook is ok for now, I will add test code like otel plugin.

Don't be afraid. Feel free to contact me once you have trouble with the hook.

spacewander avatar Aug 11 '22 06:08 spacewander

To fix https://github.com/apache/apisix/runs/7851928549?check_suite_focus=true Let's add tencent-cloud-cls to https://github.com/apache/apisix/blob/b0934f9ed45ec352c3de09f59e6912a7ba71936e/Makefile#L344

spacewander avatar Aug 16 '22 06:08 spacewander

https://github.com/apache/apisix/runs/7861163309?check_suite_focus=true Let's add the tencent-cloud-cls

spacewander avatar Aug 17 '22 01:08 spacewander

@ychensha pls don't use force-push to update code, or reviews' comments may be ignore.

tzssangglass avatar Aug 18 '22 13:08 tzssangglass

Let's merge master to make CI pass.

spacewander avatar Aug 23 '22 06:08 spacewander

As this PR is big enough, let's open another PR for the doc.

spacewander avatar Aug 24 '22 02:08 spacewander