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 basic info metrics to spin-operator #113

Closed
wants to merge 4 commits into from
Closed

Conversation

rajatjindal
Copy link
Member

@rajatjindal rajatjindal commented Feb 28, 2024

These metrics are going to be a starting point for setting up dashboards for spin app (as per #16 )

Copy link

This PR now has an image available for testing:

  ttl.sh/spoopy-operator-pr-113:24h

internal/controller/metrics.go Outdated Show resolved Hide resolved
internal/controller/spinappexecutor_controller.go Outdated Show resolved Hide resolved
@rajatjindal rajatjindal force-pushed the monitoring2 branch 2 times, most recently from 5346ee1 to 3603ab4 Compare February 29, 2024 13:02
@rajatjindal
Copy link
Member Author

Hi @endocrimes @calebschoepp - I think I have addressed all the feedback and guess this is ready for final set of review. thanks.

@endocrimes
Copy link
Contributor

I'm not convinced that this is the right way to do this style of metric (or that "info" metrics in general are something we want) - these will emit any time an app changes or at a full 10h resync, which means folks need to be pretty careful about how they use them, to a degree that I think might make them more confusing than helpful.

If all you want is a way to populate dropdowns, most dashboard building software will give you ways to do it based on more active metrics.

@rajatjindal
Copy link
Member Author

This is a gauge, so my understanding is that it will remain available irrespective of the (re) sync time.

If that is indeed the case, do you have any other concerns?

@rajatjindal rajatjindal closed this Mar 4, 2024
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