Skip to content

fix(tracing): disable tracing when no collector is configured #905

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.

Sign up for GitHub

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

Open
iamgoeldhruv wants to merge 1 commit into
base: master
Choose a base branch
from
Open

fix(tracing): disable tracing when no collector is configured #905

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions tracing/tracing.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,9 @@ type Config struct {
Endpoint string `yaml:"endpoint" json:"endpoint"`
// Insecure determines whether to use an insecure connection (no TLS)
Insecure bool `yaml:"insecure" json:"insecure"`
// Adding extra flag to check if tracing is enabled
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 9, 2026

Choose a reason for hiding this comment

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

medium

This comment is redundant as the field name Enabled is self-explanatory. In Go, comments are most effective when they clarify non-obvious aspects of the code. A more useful comment would describe the behavior when this field is not set (e.g., how it interacts with the Endpoint configuration).

Suggested change
// Adding extra flag to check if tracing is enabled
// Enabled can be used to explicitly disable tracing.

Enabled *bool `yaml:"enabled" json:"enabled"`

}

func InitTracerFromYamlConfig(ctx context.Context, config string) (*sdktrace.TracerProvider, error) {
Expand All @@ -43,6 +46,10 @@ func InitTracerFromYamlConfig(ctx context.Context, config string) (*sdktrace.Tra
// It sets up OTLP gRPC exporter, resource attributes, and W3C trace context propagation
func InitTracer(ctx context.Context, cfg Config) (*sdktrace.TracerProvider, error) {
// Validate configuration
if cfg.Enabled != nil && !*cfg.Enabled {
return nil, nil
}
Comment on lines +49 to +51
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot Jan 9, 2026

Choose a reason for hiding this comment

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

critical

This check correctly handles explicit disabling via the Enabled flag. However, it fails to address the primary goal of the PR, which is to disable tracing when no collector endpoint is configured. If a configuration omits the endpoint but does not explicitly set enabled: false, this function will still proceed and fail at the cfg.Endpoint == "" check on line 56, returning an error. This means the problem of noisy logs for endpoint-less configurations is not solved.

To fix this, the logic should be changed to disable tracing by default when cfg.Endpoint is empty. Since a complete fix requires modifying code outside of this diff hunk (specifically, the check on line 56), a direct code suggestion cannot be applied. Please consider refactoring the validation logic to ensure tracing is a no-op when Endpoint is not set.


if cfg.ServiceName == "" {
return nil, fmt.Errorf("service name is required")
}
Expand Down
17 changes: 17 additions & 0 deletions tracing/tracing_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -232,3 +232,20 @@ func TestNewTransport(t *testing.T) {
t.Error("NewTransport(customTransport) returned nil")
}
}
func TestInitTracerDisabled(t *testing.T) {
ctx := context.Background()
enabled := false

cfg := Config{
Enabled: &enabled,
}

tp, err := InitTracer(ctx, cfg)
if err != nil {
t.Fatalf("expected no error when tracing is disabled, got %v", err)
}

if tp != nil {
t.Fatalf("expected nil tracer provider when tracing is disabled")
}
}