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

Unify internal observability documentation - 2 of 3 #4322

Merged
merged 38 commits into from May 16, 2024
Merged

Unify internal observability documentation - 2 of 3 #4322

merged 38 commits into from May 16, 2024

Conversation

tiffany76
Copy link
Contributor

@tiffany76 tiffany76 commented Apr 18, 2024 •
edited

This PR is the second of three. Together they will create a new internal observability document on opentelemetry.io, combining and migrating multiple markdown files from the Collector repo. This PR covers the section on types of internal observability.

Link to tracking issue: open-telemetry/opentelemetry-collector#8886.

Also fixes #4278.

Also relates to:

Follow up PR (3 of 3) will be created once this one is merged.

PREVIEW: https://deploy-preview-4322--opentelemetry.netlify.app/docs/collector/internal-telemetry/

@tiffany76
Copy link
Contributor Author

Just getting started. I'll tag everyone when it's ready for review. Thanks!

@tiffany76 tiffany76 mentioned this pull request Apr 30, 2024
Closed
@tiffany76 tiffany76 marked this pull request as ready for review May 2, 2024 22:16
@tiffany76 tiffany76 requested review from a team as code owners May 2, 2024 22:16
@tiffany76 tiffany76 requested review from TylerHelmuth and removed request for a team May 2, 2024 22:16
@tiffany76
Copy link
Contributor Author

Hi @svrnm , @theletterf, @mx-psi, @codeboten,

This PR is ready for review. I'm also adding a few in-line comments and questions for the group. Thanks!

@tiffany76 tiffany76 mentioned this pull request May 2, 2024
Draft
@svrnm
Copy link
Member

svrnm commented May 3, 2024

This looks really good, I answered some of your questions. Thanks!

theletterf
theletterf approved these changes May 3, 2024
View reviewed changes
Copy link
Member

@theletterf theletterf left a comment

Choose a reason for hiding this comment

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

Lots of good stuff in here. Thanks so much, @tiffany76 !

content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
chalin
chalin reviewed May 7, 2024
View reviewed changes
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
chalin
chalin reviewed May 7, 2024
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.

Nice work. See inline for questions, remarks, and suggestions. Thanks.

content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
@tiffany76 tiffany76 mentioned this pull request May 8, 2024
Merged
@tiffany76
Copy link
Contributor Author

I've addressed all the comments and suggestions made so far. If anyone would like to take another look, now's your chance. Thanks!

@tiffany76 tiffany76 requested review from svrnm, mx-psi and chalin May 8, 2024 17:29
mhausenblas
mhausenblas approved these changes May 9, 2024
View reviewed changes
Copy link
Member

@mhausenblas mhausenblas left a comment

Choose a reason for hiding this comment

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

LGTM, thank you!

chalin
chalin reviewed May 9, 2024
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.

Looking good! A few more comments, see inline. Thx

content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
@svrnm svrnm added the sig-approval-missing Co-owning SIG didn't provide an approval label May 10, 2024
mx-psi
mx-psi reviewed May 10, 2024
View reviewed changes
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
tiffany76 and others added 4 commits May 13, 2024 16:16
@tiffany76
Merge branch 'main' into internal-obs-2
0920384
@tiffany76 @chalin
Update content/en/docs/collector/internal-telemetry.md
f2588c3 
Co-authored-by: Patrice Chalin <chalin@users.noreply.github.com>
@tiffany76
Merge branch 'main' into internal-obs-2
a04a6e7
@tiffany76 @mx-psi
Apply suggestions Pablo's review
ee87db1 
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
@tiffany76 tiffany76 mentioned this pull request May 14, 2024
Open
@tiffany76 tiffany76 requested a review from mx-psi May 14, 2024 22:17
@chalin
Copy link
Contributor

chalin commented May 15, 2024

Good to merge here @svrnm?

@svrnm
Copy link
Member

svrnm commented May 16, 2024

Yes!

@svrnm svrnm merged commit cf9e5b2 into open-telemetry:main May 16, 2024
15 checks passed
@chalin
Copy link
Contributor

chalin commented May 16, 2024

Great work @tiffany76 et all! 🙌🏻 🎉

@tiffany76 tiffany76 deleted the internal-obs-2 branch May 16, 2024 19:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalin chalin chalin approved these changes

@dmitryax dmitryax dmitryax left review comments

@mhausenblas mhausenblas mhausenblas approved these changes

@theletterf theletterf theletterf approved these changes

@mx-psi mx-psi mx-psi approved these changes

@TylerHelmuth TylerHelmuth Awaiting requested review from TylerHelmuth TylerHelmuth is a code owner automatically assigned from open-telemetry/collector-approvers

@svrnm svrnm Awaiting requested review from svrnm

Assignees
No one assigned
Labels
sig:collector sig-approval-missing Co-owning SIG didn't provide an approval
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

Add list of metrics by level to Collector's internal telemetry docs
7 participants
@tiffany76 @svrnm @chalin @mhausenblas @theletterf @mx-psi @dmitryax