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.

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

Add TA troubleshooting page #4708

Merged
merged 19 commits into from
Jun 23, 2024

Conversation

avillela
Copy link
Contributor

Adding page to the docs with tips for troubleshooting the OTel Operator's Target Allocator.

This was originally going to be a blog post; however, per suggestion from @svrnm, this is a much better fit.

Reference issue: #4707

@avillela avillela requested review from a team June 18, 2024 17:03
@avillela
Copy link
Contributor Author

Question for @open-telemetry/operator-approvers - I have a working example repo that I'd like to reference on this page, but I think that it's probably best to move this code to an examples folder the OTel Operator repo before doing this. Thoughts?

svrnm
svrnm previously requested changes Jun 18, 2024
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, this is a really good addition to docs! A few comments and suggestions. Note that the section titles are really long which might look strange in the outline navigation, maybe we can review and shorten them

@avillela avillela requested a review from svrnm June 18, 2024 21:29
@avillela
Copy link
Contributor Author

Thanks, this is a really good addition to docs! A few comments and suggestions. Note that the section titles are really long which might look strange in the outline navigation, maybe we can review and shorten them

Yeah, they do look a bit long. I think that when we had the step numbers, it was a bit easier to distinguish one from the other. Is there a reson why you prefer to leave those out, @svrnm?

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.

Good stuff! Added some comments.

@avillela
Copy link
Contributor Author

avillela commented Jun 19, 2024

FYI @svrnm @swiatekm-sumo @theletterf - I had some previous content for troubleshooting auto-instrumentation, so I created a page for that too.

@avillela avillela requested a review from theletterf June 19, 2024 17:19
@svrnm
Copy link
Member

svrnm commented Jun 20, 2024

FYI @svrnm @swiatekm-sumo @theletterf - I had some previous content for troubleshooting auto-instrumentation, so I creaded a page for that too.

Can you fork that out in a dedicated PR or is this closely related?

@avillela
Copy link
Contributor Author

FYI @svrnm @swiatekm-sumo @theletterf - I had some previous content for troubleshooting auto-instrumentation, so I creaded a page for that too.

Can you fork that out in a dedicated PR or is this closely related?

It's not closely related, so I can definitely create a separate PR.

@avillela
Copy link
Contributor Author

Removed auto-instrumentation troubleshooting page. I have created a separate PR for this (#4724).

@svrnm svrnm dismissed their stale review June 20, 2024 15:57

done

@svrnm
Copy link
Member

svrnm commented Jun 20, 2024

@open-telemetry/operator-approvers PTAL!

Co-authored-by: Mikołaj Świątek <[email protected]>
Copy link
Contributor Author

@avillela avillela left a comment

Choose a reason for hiding this comment

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

Applying my own updates per feedback below

@avillela avillela requested review from swiatekm and theletterf June 20, 2024 19:05
@avillela
Copy link
Contributor Author

@swiatekm-sumo thanks for keeping me honest! I've incorporated your suggestions and did another read through. Hopefully I didn't miss any spots where I mis-explained the TA functionality.

Copy link
Contributor

@jaronoff97 jaronoff97 left a comment

Choose a reason for hiding this comment

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

A few minor comments, but overall this is fantastic and will be massively helpful!

```json
{
"otelcol-collector-0": {
"_link": "/jobs/serviceMonitor%2Fopentelemetry%2Fsm-example%2F1/targets?collector_id=otelcol-collector-0",
Copy link
Contributor

Choose a reason for hiding this comment

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

would be good to note here that the query parameter collector_id is saying that these are the targets for otelcol-collector-0

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Noted!

@avillela avillela requested a review from jaronoff97 June 21, 2024 21:05
@jaronoff97
Copy link
Contributor

THANK YOU @avillela fantastic work here :D :D :D

Copy link
Contributor

@swiatekm swiatekm left a comment

Choose a reason for hiding this comment

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

Nice 💪

@cartermp cartermp merged commit 313e391 into open-telemetry:main Jun 23, 2024
16 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants