Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

feat: add opentelemetry plugin #6119

Merged
merged 11 commits into from
Jan 26, 2022
Merged

feat: add opentelemetry plugin #6119

merged 11 commits into from
Jan 26, 2022

Conversation

yangxikun
Copy link
Contributor

@yangxikun yangxikun commented Jan 16, 2022

What this PR does / why we need it:

Add opentelemetry plugin.
Fix #3891

Pre-submission checklist:

  • Did you explain what problem does this PR solve? Or what new features have been added?
  • Have you added corresponding test cases?
  • Have you modified the corresponding document?
  • Is this PR backward compatible? If it is not backward compatible, please discuss on the mailing list first

Copy link
Member

@leslie-tsang leslie-tsang left a comment

Choose a reason for hiding this comment

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

Hello there, Thanks for you contribution, Can you send a email to the mail list ?

@yangxikun yangxikun changed the title feature: add opentelemetry plugin. (#3891) feat: add opentelemetry plugin #3891 Jan 16, 2022
@xuminwlt
Copy link

great, brother.

@spacewander spacewander changed the title feat: add opentelemetry plugin #3891 feat: add opentelemetry plugin Jan 23, 2022
Copy link
Member

@bisakhmondal bisakhmondal left a comment

Choose a reason for hiding this comment

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

Great job here. Kudos to you!

@moonming
Copy link
Member

moonming commented Jan 25, 2022

@yangxikun is it possible to make an opentelemetry-lua repo like https://github.com/open-telemetry/opentelemetry-go?

Comment on lines +25 to +26
local trace_id_ratio_sampler_new =
require("opentelemetry.trace.sampling.trace_id_ratio_sampler").new
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
local trace_id_ratio_sampler_new =
require("opentelemetry.trace.sampling.trace_id_ratio_sampler").new
local trace_id_ratio_sampler_new = require("opentelemetry.trace.sampling.trace_id_ratio_sampler").new

I personally prefer to put them on the same line.

Copy link
Member

Choose a reason for hiding this comment

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

I guess it's for Linting? > 100 char

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, for linting.

@spacewander spacewander merged commit 7617e19 into apache:master Jan 26, 2022
@spacewander
Copy link
Member

Merged. Thanks!
Let's update the README too

# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

request help: opentelemetry luajit SDK and intergration with APISIX
8 participants