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

Do not panic if tracer cannot be initialized #6921

Merged
merged 1 commit into from Dec 5, 2023

Conversation

jmazzitelli
Copy link
Collaborator

fixes: #6913

To test, I just put a bogus server.observability section in the config and ran the server.

Configure it with something like this:

$ git diff
diff --git a/hack/run-kiali-config-template.yaml b/hack/run-kiali-config-template.yaml
index 83c3b5615..db4330c81 100644
--- a/hack/run-kiali-config-template.yaml
+++ b/hack/run-kiali-config-template.yaml
@@ -49,6 +49,15 @@ login_token:
 
 server:
   static_content_root_directory: "${UI_CONSOLE_DIR}"
+  observability:
+    tracing:
+      collector_type: otel
+      collector_url: http://foo:123
+      enabled: true
+      otel:
+        protocol: grpc
+        skip_verify: true
+        tls_enabled: false
 
 # in_cluster must be false - do not change
 in_cluster: false

Run the server via run-kiali.sh:

hack/run-kiali.sh -kc current

Server now starts up without panic. It does log an ERR message now saying, Failed to initialize tracer. Kiali will not log its own tracing data:

{"level":"info","time":"2023-12-05T10:14:05-05:00","message":"Adding a RegistryRefreshHandler"}
2023-12-05T10:14:05-05:00 INF Kiali: Version: v1.78.0-SNAPSHOT, Commit: 94b5b58c5a809865ee58b8a6fe0ad8064e2890b2, Go: 1.20.10
2023-12-05T10:14:05-05:00 INF Using authentication strategy [anonymous]
2023-12-05T10:14:05-05:00 WRN Kiali auth strategy is configured for anonymous access - users will not be authenticated.
2023-12-05T10:14:05-05:00 INF Tracing Enabled. Initializing tracer with collector url: http://foo:123
2023-12-05T10:14:15-05:00 ERR Failed to initialize tracer. Kiali will not log its own tracing data: context deadline exceeded
2023-12-05T10:14:15-05:00 INF Initializing Kiali Cache
2023-12-05T10:14:15-05:00 INF Adding a RegistryRefreshHandler
2023-12-05T10:14:15-05:00 INF [Kiali Cache] Waiting for cluster-scoped cache to sync
2023-12-05T10:14:16-05:00 INF [Kiali Cache] Started
2023-12-05T10:14:16-05:00 INF [Kiali Cache] Kube cache is active for cluster: [Kubernetes]
2023-12-05T10:14:16-05:00 INF Server endpoint will start at [:20001/]
2023-12-05T10:14:16-05:00 INF Server endpoint will serve static content from [/home/jmazzite/source/kiali/kiali/frontend/build]
2023-12-05T10:14:16-05:00 INF Starting Metrics Server on [:9090]

@jmazzitelli jmazzitelli self-assigned this Dec 5, 2023
@nrfox
Copy link
Contributor

nrfox commented Dec 5, 2023

Inside all a lot of the biz services they use the "global" trace provider to generates spans:

kiali/business/workloads.go

Lines 133 to 143 in 94b5b58

var end observability.EndFunc
ctx, end = observability.StartSpan(ctx, "GetWorkloadList",
observability.Attribute("package", "business"),
observability.Attribute("includeHealth", criteria.IncludeHealth),
observability.Attribute("includeIstioResources", criteria.IncludeIstioResources),
observability.Attribute("cluster", criteria.Cluster),
observability.Attribute("namespace", criteria.Namespace),
observability.Attribute("rateInterval", criteria.RateInterval),
observability.Attribute("queryTime", criteria.QueryTime),
)
defer end()

What happens when that global trace provider never gets initialized like it would here?

@nrfox
Copy link
Contributor

nrfox commented Dec 5, 2023

Inside all a lot of the biz services they use the "global" trace provider to generates spans:

kiali/business/workloads.go

Lines 133 to 143 in 94b5b58

var end observability.EndFunc
ctx, end = observability.StartSpan(ctx, "GetWorkloadList",
observability.Attribute("package", "business"),
observability.Attribute("includeHealth", criteria.IncludeHealth),
observability.Attribute("includeIstioResources", criteria.IncludeIstioResources),
observability.Attribute("cluster", criteria.Cluster),
observability.Attribute("namespace", criteria.Namespace),
observability.Attribute("rateInterval", criteria.RateInterval),
observability.Attribute("queryTime", criteria.QueryTime),
)
defer end()

What happens when that global trace provider never gets initialized like it would here?

Answering my own question... from the otel sdk it looks like it just returns a noop implementation:

// GetTracerProvider returns the registered global trace provider.
// If none is registered then an instance of NoopTracerProvider is returned.
//
// Use the trace provider to create a named tracer. E.g.
//
//	tracer := otel.GetTracerProvider().Tracer("example.com/foo")
//
// or
//
//	tracer := otel.Tracer("example.com/foo")

@jmazzitelli
Copy link
Collaborator Author

What happens when that global trace provider never gets initialized like it would here?

I assume its all OK because the code already has a check for "if the provider is nil or not enabled" -- this behaves as if it was not enabled.

https://github.com/kiali/kiali/blob/master/server/server.go#L81-L83

where tracingProvider gets set to nil here now, instead of panic:

https://github.com/kiali/kiali/blob/master/server/server.go#L35-L38

@jmazzitelli jmazzitelli merged commit 0d6ef1c into kiali:master Dec 5, 2023
6 checks passed
@jmazzitelli jmazzitelli deleted the 6913-avoid-panic branch December 5, 2023 16:18
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.

panic when observability section does not configure the tracing endpoint correctly
2 participants