Add container parser blog post #4489
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
ChrsMark
changed the title
ChrsMark
force-pushed
the
add_container_parser_blog_post
branch
from
4da29cf
to
d08fbe5
Compare
ChrsMark
force-pushed
the
add_container_parser_blog_post
branch
9 times, most recently
from
97da040
to
47528d1
Compare
svrnm
requested review from
a team and
bogdandrutu
and removed request for
a team
|
@open-telemetry/collector-approvers please take a look as well, thx! |
svrnm
requested changes
May 16, 2024
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
ChrsMark
force-pushed
the
add_container_parser_blog_post
branch
from
ad5c294
to
24a1f28
Compare
djaglowski
reviewed
May 16, 2024
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
ChrsMark
force-pushed
the
add_container_parser_blog_post
branch
4 times, most recently
from
9f7ee5b
to
83a055c
Compare
theletterf
approved these changes
May 17, 2024
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
Comment on lines
35
to
39
| operators together. The end result is rather complex, and this configuration | ||
| complexity can be mitigated by using the corresponding | ||
| [helm chart preset](https://github.com/open-telemetry/opentelemetry-helm-charts/tree/main/charts/opentelemetry-collector#configuration-for-kubernetes-container-logs). | ||
| However, despite having the preset, it can still be challenging for users to | ||
| maintain and troubleshoot such advanced configurations. |
There was a problem hiding this comment.
Suggested change
| operators together. The end result is rather complex, and this configuration | |
| complexity can be mitigated by using the corresponding | |
| [helm chart preset](https://github.com/open-telemetry/opentelemetry-helm-charts/tree/main/charts/opentelemetry-collector#configuration-for-kubernetes-container-logs). | |
| However, despite having the preset, it can still be challenging for users to | |
| maintain and troubleshoot such advanced configurations. | |
| operators together. The end result is rather complex. This configuration | |
| complexity can be mitigated by using the corresponding | |
| [helm chart preset](https://github.com/open-telemetry/opentelemetry-helm-charts/tree/main/charts/opentelemetry-collector#configuration-for-kubernetes-container-logs). | |
| However, despite having the preset, it can still be challenging for users to | |
| maintain and troubleshoot such advanced configurations. |
Style nit: In some parts you use first person plural ("we"), in others you switch to passive voice ("it can be..."). I recommend to select a single voice and stick to it, preferably second person, which is more engaging ("You can...").
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
Comment on lines
160
to
163
| In order to implement that parser operator most of the code was written from | ||
| scratch, but we were able to re-use the recombine operator internally for the | ||
| partial logs parsing. In order to achieve this some small refactoring was | ||
| required but this gave us the opportunity to re-use an already existent and well | ||
| tested component. |
There was a problem hiding this comment.
You can remove most of the "In order"s.
| required but this gave us the opportunity to re-use an already existent and well | ||
| tested component. | ||
|
|
||
| Additionally, during the discussions around the implementation of this feature, |
There was a problem hiding this comment.
Similarly, you can remove most of the opening adverbs. It'll flow better.
content/en/blog/2024/otel-collector-container-log-parser/index.md
Outdated
Show resolved
Hide resolved
ChrsMark
force-pushed
the
add_container_parser_blog_post
branch
2 times, most recently
from
ff3a2a4
to
f0033ce
Compare
| operators: | ||
| - id: container-parser | ||
| type: container | ||
| ``` |
There was a problem hiding this comment.
How about showing the comparison to the config previously (with the container parser) required (that you mention in the issue above: open-telemetry/opentelemetry-collector-contrib#31959)?
Or at least mention that the config now is 8 lines long vs. ~80 lines to achieve the same result.
There was a problem hiding this comment.
Good point! Added at 44778bd, thank's.
ChrsMark
force-pushed
the
add_container_parser_blog_post
branch
from
44778bd
to
aec5a1b
Compare
|
@svrnm @theletterf thank's for the feedback. I have tried to address your comments. Feel free to take another look. |
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
ChrsMark
force-pushed
the
add_container_parser_blog_post
branch
from
aec5a1b
to
db56d91
Compare
djaglowski
approved these changes
May 22, 2024
theletterf
approved these changes
May 22, 2024
svrnm
approved these changes
May 22, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
This PR adds a blog post about the new
filelogreceiver's container parser which was introduced with open-telemetry/opentelemetry-collector-contrib#31959./cc @djaglowski
Preview: https://deploy-preview-4489--opentelemetry.netlify.app/blog/2024/otel-collector-container-log-parser/