Skip to content

Conversation

rubenvp8510
Copy link
Collaborator

No description provided.

@rubenvp8510 rubenvp8510 force-pushed the fix_monolithic_metrics branch 2 times, most recently from f7da411 to 9da399b Compare September 11, 2025 06:18
@rubenvp8510 rubenvp8510 marked this pull request as ready for review September 11, 2025 06:19
@rubenvp8510 rubenvp8510 force-pushed the fix_monolithic_metrics branch 4 times, most recently from 840eb9e to cd9e42e Compare September 11, 2025 06:37
@rubenvp8510 rubenvp8510 force-pushed the fix_monolithic_metrics branch from cd9e42e to 171ae5a Compare September 11, 2025 06:38
@codecov-commenter
Copy link

codecov-commenter commented Sep 11, 2025

Codecov Report

❌ Patch coverage is 21.58273% with 218 lines in your changes missing coverage. Please review.
✅ Project coverage is 56.01%. Comparing base (0b0e615) to head (ea76497).
⚠️ Report is 20 commits behind head on main.

Files with missing lines Patch % Lines
internal/manifests/monolithic/certificates.go 0.00% 63 Missing ⚠️
...roller/tempo/certrotation_monolithic_controller.go 0.00% 55 Missing ⚠️
internal/manifests/monolithic/statefulset.go 13.88% 29 Missing and 2 partials ⚠️
...al/certrotation/handlers/certrotation_discovery.go 0.00% 21 Missing ⚠️
internal/manifests/monolithic/configmap.go 16.66% 19 Missing and 1 partial ⚠️
cmd/start/main.go 0.00% 8 Missing ⚠️
...nternal/certrotation/handlers/check_cert_expiry.go 0.00% 4 Missing ⚠️
internal/certrotation/var.go 20.00% 4 Missing ⚠️
internal/certrotation/handlers/options.go 0.00% 3 Missing ⚠️
internal/certrotation/handlers/rotate_certs.go 0.00% 3 Missing ⚠️
... and 2 more
Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1274      +/-   ##
==========================================
- Coverage   57.64%   56.01%   -1.64%     
==========================================
  Files         121      125       +4     
  Lines       11277    11709     +432     
==========================================
+ Hits         6501     6559      +58     
- Misses       4418     4800     +382     
+ Partials      358      350       -8     
Flag Coverage Δ
unittests 56.01% <21.58%> (-1.64%) ⬇️

Flags with carried forward coverage won't be shown. Click here to find out more.

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

config.Server.HttpServerWriteTimeout = opts.Tempo.Spec.Timeout.Duration
if tempo.Spec.Multitenancy.IsGatewayEnabled() {
// We need this to scrap metrics.
config.Server.HTTPListenAddress = "0.0.0.0"
Copy link
Collaborator

Choose a reason for hiding this comment

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

Does the internal server serve metrics, or only the readiness probes?

internal_server:
    http_listen_address: ""

Otherwise everyone has access to the Tempo API and can circumvent the gateway. That means we have to setup mTLS between tempo and the gateway also for the monolithic, which is what I tried to avoid by using localhost here.

Copy link
Collaborator Author

@rubenvp8510 rubenvp8510 Sep 12, 2025

Choose a reason for hiding this comment

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

Unfortunately internal server doesn't expose the metrics :( We will need to protect it using mTLS which is something I also don't wanted

Signed-off-by: Ruben Vargas <[email protected]>
}

// SetupWithManager sets up the controller with the Manager.
func (r *CertRotationMonolithicReconciler) SetupWithManager(mgr ctrl.Manager) error {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Where is this function being called?

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