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

OpenTelemetry Protocol with Apache Arrow in Production #5198

Open
wants to merge 25 commits into
base: main
Choose a base branch
from
Open

OpenTelemetry Protocol with Apache Arrow in Production #5198

wants to merge 25 commits into from
+425 −1

Conversation

jmacd
Copy link

@jmacd jmacd commented Sep 13, 2024

Fixes #5193.

@opentelemetrybot opentelemetrybot requested a review from a team September 13, 2024 21:43
@github-actions github-actions bot added the blog label Sep 13, 2024
@jmacd jmacd marked this pull request as ready for review September 13, 2024 21:48
@jmacd jmacd requested a review from a team September 13, 2024 21:48
tiffany76
tiffany76 reviewed Sep 13, 2024
View reviewed changes
Copy link
Contributor

@tiffany76 tiffany76 left a comment

Choose a reason for hiding this comment

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

First pass edits - had to stop around line 92. Will do more next week! Thanks.

content/en/blog/2024/otel-arrow-production/index.md Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
svrnm
svrnm requested changes Sep 16, 2024
View reviewed changes
Copy link
Member

@svrnm svrnm 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 writing this, great read. My main question is around "OTAP" vs "OTel-Arrow", are those 2 things different? So is it "OTel-Arrow Receiver" or "OTAP Receiver" like "OTLP receiver".

Also, a question on where this content should live: there is a lot of practical material in the content, that looks more like docs dann a blog. Is OTel Arrow ready for production such that we can take some of that into the documentation (e.g. within the collector docs)?

content/en/blog/2024/otel-arrow-production/index.md Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
@jmacd @tiffany76
Apply suggestions from code review
f098839 
Co-authored-by: Tiffany Hrabusa <30397949+tiffany76@users.noreply.github.com>
@opentelemetrybot opentelemetrybot requested a review from a team September 16, 2024 16:47
@jmacd jmacd mentioned this pull request Sep 16, 2024
Open
@jmacd
Copy link
Author

jmacd commented Sep 16, 2024

@svrnm About the length --
I have cut the section "Batching and back-pressure in OTel-Arrow streams", moved it to the exporter documentation: open-telemetry/opentelemetry-collector-contrib#35225

lquerel
lquerel approved these changes Sep 17, 2024
View reviewed changes
Copy link
Contributor

@lquerel lquerel left a comment

Choose a reason for hiding this comment

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

LGTM. Thanks Joshua

svrnm
svrnm requested changes Sep 17, 2024
View reviewed changes
Copy link
Member

@svrnm svrnm left a comment

Choose a reason for hiding this comment

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

lgtm overall, most comments are suggestions. There is only one around the "collaboration with F5" wording, see my inline comment

content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
@jmacd @svrnm
Apply suggestions from code review
3c1889b 
Co-authored-by: Severin Neumann <neumanns@cisco.com>
@opentelemetrybot opentelemetrybot requested a review from a team September 17, 2024 20:50
tiffany76
tiffany76 approved these changes Sep 18, 2024
View reviewed changes
Copy link
Contributor

@tiffany76 tiffany76 left a comment

Choose a reason for hiding this comment

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

Made several more copy edits, but otherwise LGTM!

content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
content/en/blog/2024/otel-arrow-production/index.md Outdated Show resolved Hide resolved
jmacd and others added 3 commits September 18, 2024 09:10
@jmacd
rephrase 2nd paragraph w/ lquerel
daf6f60
@jmacd @tiffany76
Apply suggestions from code review
fae8f4d 
Thank you @tiffany76!

Co-authored-by: Tiffany Hrabusa <30397949+tiffany76@users.noreply.github.com>
@jmacd @tiffany76
Apply suggestions from code review
06555f1 
Co-authored-by: Tiffany Hrabusa <30397949+tiffany76@users.noreply.github.com>
@tiffany76
Copy link
Contributor

/fix:format

@opentelemetrybot
Copy link
Collaborator

You triggered fix:format action run at https://github.com/open-telemetry/opentelemetry.io/actions/runs/10926671465

@opentelemetrybot
Copy link
Collaborator

fix:format was successful.

IMPORTANT: (RE-)RUN /fix:all to ensure that there are no remaining check issues.

@jmacd jmacd requested a review from a team as a code owner September 18, 2024 18:42
@opentelemetrybot opentelemetrybot requested a review from a team September 18, 2024 18:42
@jmacd
Copy link
Author

jmacd commented Sep 18, 2024

Thank you @tiffany76 and @svrnm for the helpful edits!

tiffany76
tiffany76 approved these changes Sep 18, 2024
View reviewed changes
Copy link
Contributor

@tiffany76 tiffany76 left a comment

Choose a reason for hiding this comment

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

LGTM!

@joaopgrassi
Copy link
Member

Awesome blog post! A nit only: When viewing the blog in dark mode, the image visibility isn't great. Perhaps adding a white background to it improve things.

image

jmacd
jmacd commented Sep 19, 2024
View reviewed changes
content-modules/opentelemetry-specification
Copy link
Author

Choose a reason for hiding this comment

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

I don't remember changing this file. Is it OK?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@lquerel lquerel lquerel approved these changes

@svrnm svrnm svrnm requested changes

@tiffany76 tiffany76 tiffany76 approved these changes

Requested changes must be addressed to merge this pull request.

Assignees
No one assigned
Labels
blog
Projects
SIG Comms: PRs & Issues
Status: No status
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

New Blog Post: OpenTelemetry Protocol with Apache Arrow in Production
6 participants
@jmacd @tiffany76 @opentelemetrybot @joaopgrassi @lquerel @svrnm