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 observability chart #10

Merged
merged 7 commits into from
Apr 29, 2024
Merged

Add observability chart #10

merged 7 commits into from
Apr 29, 2024

Conversation

nydr
Copy link
Contributor

@nydr nydr commented Apr 16, 2024

No description provided.

@nydr nydr requested review from Hareet and mrjones-plip April 16, 2024 10:45
Copy link
Contributor

@mrjones-plip mrjones-plip left a comment

Choose a reason for hiding this comment

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

This is over my head - made one pretty irrelevant comment, but am otherwise very much deferring to @Hareet for the real review.

@@ -0,0 +1,23 @@
# Medic Observability

## Chart
Copy link
Contributor

Choose a reason for hiding this comment

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

light preference to use mermaid instead of ASCII, but not that big of a deal. If you do convert it, build it in the live editor and then paste in the resulting markdown - v. fast!

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'm on the fence about this, mermaid if nice, but some of the consumers will be local terminal (helm show readme <chart> or locally checked out git repo) in which case I find it preferable to have plain ASCII. I vote that we keep it ASCII and reconsider mermaid if it becomes complex enough where ASCII is too limiting.


Here's an example how mermaid would look:

flowchart LR
subgraph Daemonset
NE[Node Exporter] -->|metrics| Vector
end
NL[Node Logs] -.->|logs| Vector
Opencost <-->|metrics| Prometheus
Vector -->|metrics| Prometheus
Prometheus -->|metrics| Grafana
Vector -.->|logs| Loki
Loki-.->|logs| Grafana
Loading

Copy link
Contributor

Choose a reason for hiding this comment

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

cools - Thanks for explaining your desire to go wit ASCII. makes sense then!

@nydr nydr merged commit d4c102b into main Apr 29, 2024
1 check passed
@nydr nydr deleted the dn/add_observability branch April 29, 2024 08:01
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
Development

Successfully merging this pull request may close these issues.

2 participants