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 - 1 of 3 #4246

Merged
merged 34 commits into from
Apr 17, 2024
Merged

Unify internal observability documentation - 1 of 3 #4246

merged 34 commits into from
Apr 17, 2024

Conversation

tiffany76
Copy link
Contributor

@tiffany76 tiffany76 commented Apr 4, 2024 •
edited

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

Relates to #4192. For ease of review, that larger PR is being split into three smaller ones. This is the first of three.

This PR addresses the first section of a new internal observability document on opentelemetry.io:

  • How to enable observability

Follow up PRs (2 and 3 of 3) will be created once this one is merged.

cc: @mx-psi @codeboten @theletterf @svrnm

Preview: https://deploy-preview-4246--opentelemetry.netlify.app/docs/collector/internal-telemetry/

@tiffany76
Copy link
Contributor Author

Still lots to do. Will start working on the content tomorrow.

@tiffany76 tiffany76 mentioned this pull request Apr 4, 2024
Draft
@theletterf
Copy link
Member

Thanks again, @tiffany76 ! Let us know when you want us to run a first review pass.

@tiffany76
Copy link
Contributor Author

/fix:refcache

@github-actions GitHub Actions
Copy link
Contributor

github-actions bot commented Apr 5, 2024

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

@tiffany76 tiffany76 marked this pull request as ready for review April 5, 2024 22:19
@tiffany76 tiffany76 requested review from a team as code owners April 5, 2024 22:19
@tiffany76 tiffany76 requested review from bogdandrutu and removed request for a team April 5, 2024 22:19
@tiffany76
Copy link
Contributor Author

tiffany76 commented Apr 5, 2024 •
edited

Thanks again, @tiffany76 ! Let us know when you want us to run a first review pass.

Ready for review! Make sure to view the preview because the rich diff doesn't render the tabbed panes. :)

theletterf
theletterf requested changes Apr 8, 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.

Excellent work!

I'd place this document as a sub-doc for Troubleshooting. WDYT @svrnm ?

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 Show resolved Hide resolved
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
@mx-psi mx-psi requested a review from codeboten April 8, 2024 11:53
tiffany76 and others added 4 commits April 8, 2024 11:30
@tiffany76 @theletterf @mx-psi
Apply suggestions from review
3f33443 
Co-authored-by: Fabrizio Ferri-Benedetti <fferribenedetti@splunk.com>
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
@tiffany76
Merge branch 'main' into internal-obs-1
b2ab2af
@tiffany76
Make prettier fixes
45c14f0
@tiffany76
Fix links to headings
ca60916
dmitryax
dmitryax approved these changes Apr 15, 2024
View reviewed changes
Copy link
Member

@dmitryax dmitryax left a comment

Choose a reason for hiding this comment

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

LGTM!

@dmitryax
Copy link
Member

Add profiling or not. See #4246 (comment) for context. Do Collector or docs folks have any thoughts?

My guess is that this addition can be handled in a future PR, since we are already leaning toward omitting the traces documentation here.

I agree. Profiling looks out of the scope of this work. It may be even better to utilize profiling once it's part of OTel as a new signal.

@mhausenblas
Copy link
Member

So, looks like we're almost there? Only thing to be resolved is the Grafana dashboard question, right?

@svrnm
Copy link
Member

svrnm commented Apr 16, 2024

So, looks like we're almost there? Only thing to be resolved is the Grafana dashboard question, right?

I just saw that my comments on that matter where still "pending", just submitted them!

tiffany76 and others added 3 commits April 16, 2024 11:51
@tiffany76
Copy link
Contributor Author

@svrnm I think this one is ready, but please let me know if you find any other changes I should make. Thanks!

mhausenblas
mhausenblas approved these changes Apr 16, 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.

Thank you so much @tiffany76! Let's ship it :)

mx-psi
mx-psi approved these changes Apr 16, 2024
View reviewed changes
content/en/docs/collector/internal-telemetry.md Outdated Show resolved Hide resolved
tiffany76 and others added 2 commits April 16, 2024 13:13
@tiffany76 @mx-psi
Fix default log emitting
c42680f 
Co-authored-by: Pablo Baeyens <pbaeyens31+github@gmail.com>
@tiffany76
Fix other stdout
a9af6e3
@svrnm svrnm merged commit b0713c8 into open-telemetry:main Apr 17, 2024
14 checks passed
@svrnm
Copy link
Member

svrnm commented Apr 17, 2024

🎉 thank you @tiffany76

@tiffany76 tiffany76 deleted the internal-obs-1 branch April 17, 2024 23:58
@tiffany76 tiffany76 mentioned this pull request Apr 18, 2024
Merged
ricardoamaro pushed a commit to ricardoamaro/opentelemetry.io that referenced this pull request Apr 19, 2024
@tiffany76 @ricardoamaro
Unify internal observability documentation - 1 of 3 (open-telemetry#4246
497212f 
)
chalin
chalin reviewed May 7, 2024
View reviewed changes
content/en/docs/collector/internal-telemetry.md
Comment on lines +83 to +84
Self-monitoring is a risky practice. If an issue arises, the source of the
problem is unclear and the telemetry is unreliable.
Copy link
Contributor

Choose a reason for hiding this comment

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

Late to the party, but this caution text doesn't make sense / is incomplete.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I changed the caution text in #4322. Please let me know if the new alert makes more sense.

Copy link
Contributor

Choose a reason for hiding this comment

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

Great, thanks! I'll add comments, if any in #4322.

@tiffany76 tiffany76 mentioned this pull request May 23, 2024
Open
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@chalin chalin chalin left review comments

@mhausenblas mhausenblas mhausenblas approved these changes

@svrnm svrnm svrnm approved these changes

@theletterf theletterf theletterf approved these changes

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

@dmitryax dmitryax dmitryax approved these changes

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

@codeboten codeboten Awaiting requested review from codeboten

Assignees
No one assigned
Labels
sig:collector
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
8 participants
@tiffany76 @theletterf @svrnm @mhausenblas @dmitryax @chalin @mx-psi @opentelemetrybot