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 diagrams to further enhance Target Allocator documentation - #2229 #2230

Merged
merged 8 commits into from
Oct 17, 2023

Conversation

avillela
Copy link
Contributor

Description: Add diagrams to further clarify Target Allocator functionality

Link to tracking Issue: #2229

Testing: n/a

Documentation: Updated Target Allocator readme and added further clarification to turning on multi-container multi-instrumentation functionality in main readme.

@avillela avillela requested review from a team October 13, 2023 14:09
cmd/otel-allocator/README.md Outdated Show resolved Hide resolved
cmd/otel-allocator/README.md Outdated Show resolved Hide resolved
Copy link
Member

Choose a reason for hiding this comment

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

The collector directly scrapes the metric targets directly. The diagram implies that the target allocator proxies the requests.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

TBH, I was struggling a bit with how to depict this...do you have any suggestions for improvement?

Copy link
Member

Choose a reason for hiding this comment

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

This is how would do it, tough I would wait for a second opinion on this.

Merge both graphics into one.
The service_discovery looks good, so you can use it as basis

From ServiceMonitor and PodMonitor draw arrows to represent that they are referencing the metric targets and from the receivers draw arrows to the targets too. This represents the scrape requests.
The graphic would also benefit arrow descriptions.

If this are mermaid graphics, you should also add them to the repository so they can be adapted later on.

I hope this helps.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks @secustor. I also chatted with @jaronoff97 and came up with a revised version of the first diagram. I also put them into Mermaid to make it easier to edit. If these are okay, I can delete the image files.

Copy link
Member

Choose a reason for hiding this comment

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

LGTM

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Removed the images in favor of the mermaid diagrams.

@avillela avillela requested a review from secustor October 13, 2023 21:34
@jaronoff97 jaronoff97 merged commit b546584 into open-telemetry:main Oct 17, 2023
24 checks passed
ItielOlenick pushed a commit to ItielOlenick/opentelemetry-operator that referenced this pull request May 1, 2024
…-telemetry#2229 (open-telemetry#2230)

* Add diagrams to further enhance Target Allocator documentation - open-telemetry#2229

* Update cmd/otel-allocator/README.md

Co-authored-by: Sebastian Poxhofer <[email protected]>

* Update cmd/otel-allocator/README.md

Co-authored-by: Sebastian Poxhofer <[email protected]>

* Update TA diagram

* Update TA diagram and description

* Add mermaid diagrams

* Add mermaid diagrams

* Remove diagram images (using mermaid instead)

---------

Co-authored-by: Sebastian Poxhofer <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants