Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
2 changes: 2 additions & 0 deletions apps/chproxy/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,2 @@
chproxy
coverage.out
5 changes: 4 additions & 1 deletion apps/chproxy/config.go
Original file line number Diff line number Diff line change
Expand Up @@ -29,11 +29,14 @@ func LoadConfig() (*Config, error) {
ListenerPort: "7123",
LogDebug: false,
ServiceName: "chproxy",
ServiceVersion: "1.3.0",
ServiceVersion: "1.3.1",
TraceMaxBatchSize: 512,
TraceSampleRate: 0.25, // Sample 25%
}

// Generic logger
config.Logger = slog.New(slog.NewTextHandler(os.Stdout, nil))

config.ClickhouseURL = os.Getenv("CLICKHOUSE_URL")
if config.ClickhouseURL == "" {
return nil, fmt.Errorf("CLICKHOUSE_URL must be defined")
Expand Down
244 changes: 244 additions & 0 deletions apps/chproxy/config_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,244 @@
package main

import (
"os"
"testing"
"time"
)

func TestLoadConfig(t *testing.T) {
// Setup and cleanup for all tests
setupEnv := func(t *testing.T, envs map[string]string) {
t.Helper()
for k, v := range envs {
os.Setenv(k, v)
}
}

cleanEnv := func(t *testing.T, keys []string) {
t.Helper()
for _, k := range keys {
os.Unsetenv(k)
}
}

// Required env vars that need to be cleaned up
envKeys := []string{
"CLICKHOUSE_URL",
"BASIC_AUTH",
"PORT",
"OTEL_EXPORTER_LOG_DEBUG",
"OTEL_TRACE_SAMPLE_RATE",
}

// Cleanup after all tests in this function
t.Cleanup(func() {
cleanEnv(t, envKeys)
})

// Test for valid config with default values
t.Run("ValidConfig", func(t *testing.T) {
// Set required environment variables
setupEnv(t, map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
"BASIC_AUTH": "user:password",
})

config, err := LoadConfig()
if err != nil {
t.Fatalf("LoadConfig() error = %v, wantErr = false", err)
}

// Use a table-driven check for expected values
tests := []struct {
name string
got interface{}
expected interface{}
}{
{"FlushInterval", config.FlushInterval, time.Second * 5},
{"ListenerPort", config.ListenerPort, "7123"},
{"LogDebug", config.LogDebug, false},
{"ServiceName", config.ServiceName, "chproxy"},
{"ServiceVersion", config.ServiceVersion, "1.3.1"},
{"TraceMaxBatchSize", config.TraceMaxBatchSize, 512},
{"TraceSampleRate", config.TraceSampleRate, 0.25},
{"ClickhouseURL", config.ClickhouseURL, "http://localhost:8123"},
{"BasicAuth", config.BasicAuth, "user:password"},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
if tc.got != tc.expected {
t.Errorf("%s = %v, want %v", tc.name, tc.got, tc.expected)
}
})
}

if config.Logger == nil {
t.Error("Logger is nil, want non-nil")
}
})

// Test for error cases
t.Run("ErrorCases", func(t *testing.T) {
tests := []struct {
name string
envs map[string]string
wantErr string
}{
{
name: "MissingClickhouseURL",
envs: map[string]string{
"BASIC_AUTH": "user:password",
},
wantErr: "CLICKHOUSE_URL must be defined",
},
{
name: "MissingBasicAuth",
envs: map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
},
wantErr: "BASIC_AUTH must be defined",
},
{
name: "InvalidSampleRate",
envs: map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
"BASIC_AUTH": "user:password",
"OTEL_TRACE_SAMPLE_RATE": "invalid",
},
wantErr: "invalid TRACE_SAMPLE_RATE",
},
{
name: "SampleRateTooLow",
envs: map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
"BASIC_AUTH": "user:password",
"OTEL_TRACE_SAMPLE_RATE": "-0.1",
},
wantErr: "OTEL_TRACE_SAMPLE_RATE must be between 0.0 and 1.0",
},
{
name: "SampleRateTooHigh",
envs: map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
"BASIC_AUTH": "user:password",
"OTEL_TRACE_SAMPLE_RATE": "1.1",
},
wantErr: "OTEL_TRACE_SAMPLE_RATE must be between 0.0 and 1.0",
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Clean environment before each test case
cleanEnv(t, envKeys)

// Set up environment for this test case
setupEnv(t, tc.envs)

_, err := LoadConfig()
if err == nil {
t.Fatalf("LoadConfig() error = nil, wantErr = true")
}

if got := err.Error(); got != tc.wantErr && got[:len(tc.wantErr)] != tc.wantErr {
t.Errorf("LoadConfig() error = %v, want to contain %v", got, tc.wantErr)
Comment on lines +145 to +146
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Potential index out of range error in error comparison

The current error check could panic if the actual error message is shorter than the expected error message.

Replace with a safer approach using strings.Contains:

- if got := err.Error(); got != tc.wantErr && got[:len(tc.wantErr)] != tc.wantErr {
-   t.Errorf("LoadConfig() error = %v, want to contain %v", got, tc.wantErr)
- }
+ import "strings" // Add this to imports
+ if got := err.Error(); got != tc.wantErr && !strings.Contains(got, tc.wantErr) {
+   t.Errorf("LoadConfig() error = %v, want to contain %v", got, tc.wantErr)
+ }
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
if got := err.Error(); got != tc.wantErr && got[:len(tc.wantErr)] != tc.wantErr {
t.Errorf("LoadConfig() error = %v, want to contain %v", got, tc.wantErr)
// Add the following to your imports section if it's not already present:
import "strings"
// ... (other code)
if got := err.Error(); got != tc.wantErr && !strings.Contains(got, tc.wantErr) {
t.Errorf("LoadConfig() error = %v, want to contain %v", got, tc.wantErr)
}

}
})
}
})

// Test for custom configurations
t.Run("CustomConfigurations", func(t *testing.T) {
// Table-driven tests for custom configurations
tests := []struct {
name string
envs map[string]string
checkFunc func(*testing.T, *Config)
}{
{
name: "CustomPort",
envs: map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
"BASIC_AUTH": "user:password",
"PORT": "8000",
},
checkFunc: func(t *testing.T, c *Config) {
if c.ListenerPort != "8000" {
t.Errorf("ListenerPort = %s, want 8000", c.ListenerPort)
}
},
},
{
name: "DebugMode",
envs: map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
"BASIC_AUTH": "user:password",
"OTEL_EXPORTER_LOG_DEBUG": "true",
},
checkFunc: func(t *testing.T, c *Config) {
if !c.LogDebug {
t.Errorf("LogDebug = %v, want true", c.LogDebug)
}
},
},
{
name: "CustomSampleRate",
envs: map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
"BASIC_AUTH": "user:password",
"OTEL_TRACE_SAMPLE_RATE": "0.5",
},
checkFunc: func(t *testing.T, c *Config) {
if c.TraceSampleRate != 0.5 {
t.Errorf("TraceSampleRate = %f, want 0.5", c.TraceSampleRate)
}
},
},
{
name: "ZeroSampleRate",
envs: map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
"BASIC_AUTH": "user:password",
"OTEL_TRACE_SAMPLE_RATE": "0.0",
},
checkFunc: func(t *testing.T, c *Config) {
if c.TraceSampleRate != 0.0 {
t.Errorf("TraceSampleRate = %f, want 0.0", c.TraceSampleRate)
}
},
},
{
name: "OneSampleRate",
envs: map[string]string{
"CLICKHOUSE_URL": "http://localhost:8123",
"BASIC_AUTH": "user:password",
"OTEL_TRACE_SAMPLE_RATE": "1.0",
},
checkFunc: func(t *testing.T, c *Config) {
if c.TraceSampleRate != 1.0 {
t.Errorf("TraceSampleRate = %f, want 1.0", c.TraceSampleRate)
}
},
},
}

for _, tc := range tests {
t.Run(tc.name, func(t *testing.T) {
// Clean environment before each test case
cleanEnv(t, envKeys)

// Set up environment for this test case
setupEnv(t, tc.envs)

config, err := LoadConfig()
if err != nil {
t.Fatalf("LoadConfig() error = %v, wantErr = false", err)
}

tc.checkFunc(t, config)
})
}
})
}
6 changes: 4 additions & 2 deletions apps/chproxy/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,8 @@ var (
func main() {
config, err := LoadConfig()
if err != nil {
log.Fatalf("failed to load configuration: %v", err)
config.Logger.Error("failed to load configuration", slog.String("error", err.Error()))
os.Exit(1)
}

httpClient = &http.Client{
Expand All @@ -51,7 +52,8 @@ func main() {
var cleanup func(context.Context) error
telemetry, cleanup, err = setupTelemetry(ctx, config)
if err != nil {
log.Fatalf("failed to setup telemetry: %v", err)
config.Logger.Error("failed to setup telemetry", slog.String("error", err.Error()))
os.Exit(1)
}
defer func() {
cleanupCtx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
Expand Down
2 changes: 1 addition & 1 deletion apps/chproxy/otel.go
Original file line number Diff line number Diff line change
Expand Up @@ -141,7 +141,7 @@ func setupTelemetry(ctx context.Context, config *Config) (*TelemetryConfig, func
}

config.Logger.Info("configured tracer with sampling",
"rate", config.TraceSampleRate)
slog.Float64("rate", config.TraceSampleRate))

traceProvider := sdktrace.NewTracerProvider(
sdktrace.WithResource(res),
Expand Down
Loading