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

Jump to bottom

blog post: add OpenTelemetry-Apache APISIX integration blog #1185

Merged
merged 20 commits into from
Mar 26, 2022
Merged

blog post: add OpenTelemetry-Apache APISIX integration blog #1185

merged 20 commits into from
Mar 26, 2022

Conversation

yzeng25
Copy link
Contributor

@yzeng25 yzeng25 commented Mar 3, 2022 •
edited
Loading

As discussed in #1076 , the blog and related images are uploaded in this pull request.

  1. Add markdown text contents for the blog.
  2. Add .png images for the blog in static/img/apisix directory.
  3. Register the plugin in /content/en/registry by creating a new file named instrumentation-lua-apache-apisix.md according to the template.

Preview: https://deploy-preview-1185--opentelemetry.netlify.app/blog/2022/apisix/

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Mar 3, 2022 •
edited
Loading

CLA Signed

The committers are authorized under a signed CLA.

@yzeng25 yzeng25 changed the title docs: add OpenTelemetry-Apache APISIX integration blog doc: add OpenTelemetry-Apache APISIX integration blog Mar 3, 2022
@yzeng25 yzeng25 changed the title doc: add OpenTelemetry-Apache APISIX integration blog blog post: add OpenTelemetry-Apache APISIX integration blog Mar 3, 2022
@yzeng25 yzeng25 marked this pull request as ready for review March 3, 2022 11:33
@yzeng25 yzeng25 requested a review from a team March 3, 2022 11:33
@chalin
Copy link
Contributor

chalin commented Mar 3, 2022

/assign @open-telemetry/blog-approvers

chalin
chalin reviewed Mar 3, 2022
View reviewed changes
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Thanks for your submission. I've assigned this to the blog approvers. I'll also be reviewing it soon. In the meantime, could you please line-wrap all paragraphs to 80 characters? Thanks.

SergeyKanzhelev
SergeyKanzhelev reviewed Mar 3, 2022
View reviewed changes
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

thank you for writing. I left a few suggestions. I think the most important suggestion is to explain more why this plugin is useful and what scenarios it solves. Adding this information will make this more of a blog post and less of the instruction.

Thanks again, I really impressed with the step-by-step instructions and how easy to understand every step.

content/en/blog/2022/apisix.md Outdated

The `opentelemetry` plugin of Apache APISIX implements Tracing data collection based on OpenTelemetry native standard (OTLP/HTTP), and sends it to OpenTelemetry Collector through HTTP protocol. This feature will be supported online in Apache APISIX 2.13.0.

Since the Agent/SDK of OpenTelemetry has nothing to do with the back-end implementation, when the application integrates the Agent/SDK of OpenTelemetry, the user can easily and freely change the observability backend service without any perception on the application side, such as switching from Zipkin into Jaeger.
Copy link
Member

Choose a reason for hiding this comment

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

does this sentence talking about opentelemetry in general or implies that the APISIX plugin implements the Zipkin protocol? It is a bit confusing, maybe clarify

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Clarified.

content/en/blog/2022/apisix.md

![Zipkin UI](/img/apisix/zipkin-ui-2.png)

## Disable the Plugin
Copy link
Member

Choose a reason for hiding this comment

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

please add what will change if it's disabled. Both spans (client and server) will still be connected, but APISIX span will be missing, correct?

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. Add a sentence to clarify.

@yzeng25
Copy link
Contributor Author

yzeng25 commented Mar 4, 2022

Hi @chalin @SergeyKanzhelev ,

Thanks for your reviews, I will update accordingly and have it done by earlier next week.

@austinlparker
Copy link
Member

@yzeng25 are the edits here finished?

@yzeng25
Copy link
Contributor Author

yzeng25 commented Mar 24, 2022

@yzeng25 are the edits here finished?

Yes. Please take a look when available, thanks :)

SergeyKanzhelev
SergeyKanzhelev reviewed Mar 24, 2022
View reviewed changes
Copy link
Member

@SergeyKanzhelev SergeyKanzhelev left a comment

Choose a reason for hiding this comment

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

lgtm except minor neats

chalin
chalin requested changes Mar 24, 2022
View reviewed changes
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Coming along well! See inline comments, thanks.

chalin added a commit to chalin/apisix that referenced this pull request Mar 24, 2022
@chalin
OpenTelemetry plugin: fix links to specification
02e1374 
Context: open-telemetry/opentelemetry.io#1185

/cc @austinlparker
@chalin chalin mentioned this pull request Mar 24, 2022
Merged
chalin
chalin reviewed Mar 25, 2022
View reviewed changes
chalin
chalin reviewed Mar 25, 2022
View reviewed changes
chalin
chalin reviewed Mar 25, 2022
View reviewed changes
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Thanks for noticing the .DS_Store files @leslie-tsang.

Btw @yzeng25, there's one file left:

image

@yzeng25
Copy link
Contributor Author

yzeng25 commented Mar 25, 2022

Thanks for noticing the .DS_Store files @leslie-tsang.

Btw @yzeng25, there's one file left:

image

It takes a while but I have it removed now.

image

@chalin chalin mentioned this pull request Mar 25, 2022
Merged
chalin
chalin approved these changes Mar 26, 2022
View reviewed changes
Copy link
Contributor

@chalin chalin left a comment

Choose a reason for hiding this comment

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

Thanks to all for your efforts in reviewing this blog post, and thanks to @yzeng25 for making the numerous updates.

Based on @SergeyKanzhelev's approval (aside from minor edits), I'm going to merge this now.

@chalin chalin merged commit 17f9ab7 into open-telemetry:main Mar 26, 2022
@yzeng25
Copy link
Contributor Author

yzeng25 commented Mar 28, 2022

Thanks @chalin @SergeyKanzhelev @leslie-tsang for reviews and comments!

@yzeng25 yzeng25 deleted the apisix_blog branch March 28, 2022 10:32
@yzeng25 yzeng25 mentioned this pull request Mar 31, 2022
Merged
# for free to join this conversation on GitHub. Already have an account? # to comment
Reviewers

@SergeyKanzhelev SergeyKanzhelev SergeyKanzhelev left review comments

@leslie-tsang leslie-tsang leslie-tsang left review comments

@chalin chalin chalin approved these changes

Assignees

@alolita alolita

@austinlparker austinlparker

@SergeyKanzhelev SergeyKanzhelev

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@yzeng25 @chalin @austinlparker @SergeyKanzhelev @leslie-tsang @alolita