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

[tempo] feat: add readiness and liveness probe #3188

Conversation

Mistral-valaise
Copy link
Contributor

this change resolves #3185 by adding readiness and liveness probe configuration options to tempo helm chart

@Mistral-valaise Mistral-valaise force-pushed the feature/Add-Readiness-and-Liveness-Probe-Configuration-Options-to-Tempo-Helm-Chart branch from 22c8af4 to 2c17ffd Compare June 26, 2024 05:41
@CLAassistant
Copy link

CLAassistant commented Jun 26, 2024

CLA assistant check
All committers have signed the CLA.

@CLAassistant
Copy link

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@Mistral-valaise Mistral-valaise force-pushed the feature/Add-Readiness-and-Liveness-Probe-Configuration-Options-to-Tempo-Helm-Chart branch from 1832853 to 3d952af Compare June 27, 2024 08:03
@zanhsieh zanhsieh changed the title Feature/add readiness and liveness probe configuration options to tempo helm chart [tempo] feat: add readiness and liveness probe Aug 28, 2024
@rim99
Copy link

rim99 commented Dec 7, 2024

What's the issue with this PR?
How can we merge this?

@Mistral-valaise
Copy link
Contributor Author

i did it more than 5 months ago but unfortunately it was not reviewed. i will update the branch and validate all again

@Mistral-valaise Mistral-valaise requested a review from a team as a code owner December 7, 2024 19:45
@joe-elliott
Copy link
Member

thx for the PR! can you review the linting issues?

Sheikh-Abubaker and others added 6 commits December 12, 2024 17:58
Signed-off-by: Sheikh-Abubaker <[email protected]>
Signed-off-by: Sheikh-Abubaker <[email protected]>
 to 0.50

Signed-off-by: drfaust92 <[email protected]>
 to 0.50

Signed-off-by: drfaust92 <[email protected]>
Signed-off-by: AvivGuiser <[email protected]>
CostisC and others added 5 commits December 12, 2024 18:02
…s` objects.

There's been recent confusion over the use of the `global_overrides` and
`overrides` objects. This PR aims to make it clearer.

Signed-off-by: Heds Simons <[email protected]>
Signed-off-by: xogoodnow <[email protected]>
@Mistral-valaise Mistral-valaise force-pushed the feature/Add-Readiness-and-Liveness-Probe-Configuration-Options-to-Tempo-Helm-Chart branch from 98974ff to 73429dc Compare December 12, 2024 17:03
@Mistral-valaise Mistral-valaise requested review from mapno and a team as code owners December 12, 2024 17:03
@Mistral-valaise
Copy link
Contributor Author

i made the last 2 commits without signoff, since i had merged the main branch to mine afterwards, i have difficulties to validate DCO afterwards.
I could continue working on it during the weekend.
Can I create a new branch and do it again?

@Sheikh-Abubaker
Copy link
Collaborator

i made the last 2 commits without signoff, since i had merged the main branch to mine afterwards, i have difficulties to validate DCO afterwards. I could continue working on it during the weekend. Can I create a new branch and do it again?

Sure! you could open a new fresh PR.

@Mistral-valaise
Copy link
Contributor Author

After reviewing the Grafana Tempo documentation (https://grafana.com/docs/tempo/latest/api_docs/#readiness-probe), I found that the /ready endpoint is available for readiness probes. However, there isn’t a specific endpoint designated for liveness probes. To address this, I propose using the /ready endpoint for the readiness probe and the /metrics endpoint for the liveness probe. Could you please share your thoughts on this approach?
Thank you for your consideration.
e.g
livenessProbe:
httpGet:
path: /metrics
port: 3100
initialDelaySeconds: 30
periodSeconds: 10

readinessProbe:
httpGet:
path: /ready
port: 3100
initialDelaySeconds: 5
periodSeconds: 10

@Mistral-valaise
Copy link
Contributor Author

here is my new PR #3489

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.

[Tempo] Add Readiness and Liveness Probe Configuration Options to Tempo Helm Chart