update as per the PR comment

This commit is contained in:
Manendra Pal Singh
2025-12-02 23:00:28 +05:30
parent 8fd7afb54a
commit 045f29da13
11 changed files with 128 additions and 187 deletions

View File

@@ -25,7 +25,7 @@ import (
type Config struct {
AppName string `yaml:"appName"`
Log log.Config `yaml:"log"`
Telemetry *telemetry.Config `yaml:"telemetry"`
Telemetry *otelsetup.Config `yaml:"telemetry"`
PluginManager *plugin.ManagerConfig `yaml:"pluginManager"`
Modules []module.Config `yaml:"modules"`
HTTP httpConfig `yaml:"http"`
@@ -95,7 +95,7 @@ func validateConfig(cfg *Config) error {
}
// initPlugins initializes application-level plugins including telemetry.
func initPlugins(ctx context.Context, mgr *plugin.Manager, telemetryCfg *telemetry.Config) (*telemetry.Provider, error) {
func initPlugins(ctx context.Context, mgr *plugin.Manager, telemetryCfg *otelsetup.Config) (*telemetry.Provider, error) {
if telemetryCfg == nil {
log.Info(ctx, "Telemetry config not provided; skipping OpenTelemetry setup")
return nil, nil

View File

@@ -5,13 +5,14 @@ import (
"net/http/httptest"
"testing"
"github.com/beckn-one/beckn-onix/pkg/telemetry"
"github.com/beckn-one/beckn-onix/pkg/plugin/implementation/otelsetup"
"github.com/stretchr/testify/require"
)
func TestMetricsEndpointExposesPrometheus(t *testing.T) {
ctx := context.Background()
provider, err := telemetry.NewProvider(ctx, &telemetry.Config{
setup := otelsetup.Setup{}
provider, err := setup.New(ctx, &otelsetup.Config{
ServiceName: "test-onix",
ServiceVersion: "1.0.0",
EnableMetrics: true,

View File

@@ -22,8 +22,8 @@ func (m metricsProvider) New(ctx context.Context, config map[string]string) (*te
return nil, nil, errors.New("context cannot be nil")
}
// Convert map[string]string to telemetry.Config
telemetryConfig := &telemetry.Config{
// Convert map[string]string to otelsetup.Config
telemetryConfig := &otelsetup.Config{
ServiceName: config["serviceName"],
ServiceVersion: config["serviceVersion"],
Environment: config["environment"],
@@ -44,13 +44,13 @@ func (m metricsProvider) New(ctx context.Context, config map[string]string) (*te
// Apply defaults if fields are empty
if telemetryConfig.ServiceName == "" {
telemetryConfig.ServiceName = telemetry.DefaultConfig().ServiceName
telemetryConfig.ServiceName = otelsetup.DefaultConfig().ServiceName
}
if telemetryConfig.ServiceVersion == "" {
telemetryConfig.ServiceVersion = telemetry.DefaultConfig().ServiceVersion
telemetryConfig.ServiceVersion = otelsetup.DefaultConfig().ServiceVersion
}
if telemetryConfig.Environment == "" {
telemetryConfig.Environment = telemetry.DefaultConfig().Environment
telemetryConfig.Environment = otelsetup.DefaultConfig().Environment
}
log.Debugf(ctx, "Telemetry config mapped: %+v", telemetryConfig)

View File

@@ -4,7 +4,7 @@ import (
"context"
"testing"
"github.com/beckn-one/beckn-onix/pkg/telemetry"
"github.com/beckn-one/beckn-onix/pkg/plugin/implementation/otelsetup"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -18,8 +18,8 @@ func TestMetricsProviderNew_Success(t *testing.T) {
config map[string]string
}{
{
name: "Valid config with all fields",
ctx: context.Background(),
name: "Valid config with all fields",
ctx: context.Background(),
config: map[string]string{
"serviceName": "test-service",
"serviceVersion": "1.0.0",
@@ -33,15 +33,15 @@ func TestMetricsProviderNew_Success(t *testing.T) {
config: map[string]string{},
},
{
name: "Valid config with enableMetrics false",
ctx: context.Background(),
name: "Valid config with enableMetrics false",
ctx: context.Background(),
config: map[string]string{
"enableMetrics": "false",
},
},
{
name: "Valid config with partial fields",
ctx: context.Background(),
name: "Valid config with partial fields",
ctx: context.Background(),
config: map[string]string{
"serviceName": "custom-service",
"serviceVersion": "2.0.0",
@@ -105,7 +105,7 @@ func TestMetricsProviderNew_ConfigConversion(t *testing.T) {
tests := []struct {
name string
config map[string]string
expectedConfig *telemetry.Config
expectedConfig *otelsetup.Config
}{
{
name: "All fields provided",
@@ -115,7 +115,7 @@ func TestMetricsProviderNew_ConfigConversion(t *testing.T) {
"enableMetrics": "true",
"environment": "production",
},
expectedConfig: &telemetry.Config{
expectedConfig: &otelsetup.Config{
ServiceName: "my-service",
ServiceVersion: "3.0.0",
EnableMetrics: true,
@@ -125,11 +125,11 @@ func TestMetricsProviderNew_ConfigConversion(t *testing.T) {
{
name: "Empty config uses defaults",
config: map[string]string{},
expectedConfig: &telemetry.Config{
ServiceName: telemetry.DefaultConfig().ServiceName,
ServiceVersion: telemetry.DefaultConfig().ServiceVersion,
expectedConfig: &otelsetup.Config{
ServiceName: otelsetup.DefaultConfig().ServiceName,
ServiceVersion: otelsetup.DefaultConfig().ServiceVersion,
EnableMetrics: true, // Default when not specified
Environment: telemetry.DefaultConfig().Environment,
Environment: otelsetup.DefaultConfig().Environment,
},
},
{
@@ -137,11 +137,11 @@ func TestMetricsProviderNew_ConfigConversion(t *testing.T) {
config: map[string]string{
"enableMetrics": "false",
},
expectedConfig: &telemetry.Config{
ServiceName: telemetry.DefaultConfig().ServiceName,
ServiceVersion: telemetry.DefaultConfig().ServiceVersion,
expectedConfig: &otelsetup.Config{
ServiceName: otelsetup.DefaultConfig().ServiceName,
ServiceVersion: otelsetup.DefaultConfig().ServiceVersion,
EnableMetrics: false,
Environment: telemetry.DefaultConfig().Environment,
Environment: otelsetup.DefaultConfig().Environment,
},
},
{
@@ -149,11 +149,11 @@ func TestMetricsProviderNew_ConfigConversion(t *testing.T) {
config: map[string]string{
"enableMetrics": "invalid",
},
expectedConfig: &telemetry.Config{
ServiceName: telemetry.DefaultConfig().ServiceName,
ServiceVersion: telemetry.DefaultConfig().ServiceVersion,
expectedConfig: &otelsetup.Config{
ServiceName: otelsetup.DefaultConfig().ServiceName,
ServiceVersion: otelsetup.DefaultConfig().ServiceVersion,
EnableMetrics: true, // Defaults to true on parse error
Environment: telemetry.DefaultConfig().Environment,
Environment: otelsetup.DefaultConfig().Environment,
},
},
}
@@ -308,4 +308,3 @@ func TestMetricsProviderNew_DefaultValues(t *testing.T) {
assert.NoError(t, err, "cleanup() should not return error")
}
}

View File

@@ -22,8 +22,26 @@ import (
// behind the MetricsProvider interface.
type Setup struct{}
// ToPluginConfig converts telemetry.Config to plugin.Config format.
func ToPluginConfig(cfg *telemetry.Config) *plugin.Config {
// Config represents OpenTelemetry related configuration.
type Config struct {
ServiceName string `yaml:"serviceName"`
ServiceVersion string `yaml:"serviceVersion"`
EnableMetrics bool `yaml:"enableMetrics"`
Environment string `yaml:"environment"`
}
// DefaultConfig returns sensible defaults for telemetry configuration.
func DefaultConfig() *Config {
return &Config{
ServiceName: "beckn-onix",
ServiceVersion: "dev",
EnableMetrics: true,
Environment: "development",
}
}
// ToPluginConfig converts Config to plugin.Config format.
func ToPluginConfig(cfg *Config) *plugin.Config {
return &plugin.Config{
ID: "otelsetup",
Config: map[string]string{
@@ -38,20 +56,20 @@ func ToPluginConfig(cfg *telemetry.Config) *plugin.Config {
// New initializes the underlying telemetry provider. The returned provider
// exposes the HTTP handler and shutdown hooks that the core application can
// manage directly.
func (Setup) New(ctx context.Context, cfg *telemetry.Config) (*telemetry.Provider, error) {
func (Setup) New(ctx context.Context, cfg *Config) (*telemetry.Provider, error) {
if cfg == nil {
return nil, fmt.Errorf("telemetry config cannot be nil")
}
// Apply defaults if fields are empty
if cfg.ServiceName == "" {
cfg.ServiceName = telemetry.DefaultConfig().ServiceName
cfg.ServiceName = DefaultConfig().ServiceName
}
if cfg.ServiceVersion == "" {
cfg.ServiceVersion = telemetry.DefaultConfig().ServiceVersion
cfg.ServiceVersion = DefaultConfig().ServiceVersion
}
if cfg.Environment == "" {
cfg.Environment = telemetry.DefaultConfig().Environment
cfg.Environment = DefaultConfig().Environment
}
if !cfg.EnableMetrics {

View File

@@ -6,7 +6,6 @@ import (
"net/http/httptest"
"testing"
"github.com/beckn-one/beckn-onix/pkg/telemetry"
"github.com/stretchr/testify/assert"
"github.com/stretchr/testify/require"
)
@@ -17,11 +16,11 @@ func TestSetup_New_Success(t *testing.T) {
tests := []struct {
name string
cfg *telemetry.Config
cfg *Config
}{
{
name: "Valid config with all fields",
cfg: &telemetry.Config{
cfg: &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: true,
@@ -30,7 +29,7 @@ func TestSetup_New_Success(t *testing.T) {
},
{
name: "Valid config with metrics disabled",
cfg: &telemetry.Config{
cfg: &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: false,
@@ -39,7 +38,7 @@ func TestSetup_New_Success(t *testing.T) {
},
{
name: "Config with empty fields uses defaults",
cfg: &telemetry.Config{
cfg: &Config{
ServiceName: "",
ServiceVersion: "",
EnableMetrics: true,
@@ -82,7 +81,7 @@ func TestSetup_New_Failure(t *testing.T) {
tests := []struct {
name string
cfg *telemetry.Config
cfg *Config
wantErr bool
}{
{
@@ -112,7 +111,7 @@ func TestSetup_New_DefaultValues(t *testing.T) {
ctx := context.Background()
// Test with empty fields - should use defaults
cfg := &telemetry.Config{
cfg := &Config{
ServiceName: "",
ServiceVersion: "",
EnableMetrics: true,
@@ -142,7 +141,7 @@ func TestSetup_New_MetricsDisabled(t *testing.T) {
setup := Setup{}
ctx := context.Background()
cfg := &telemetry.Config{
cfg := &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: false,
@@ -165,13 +164,13 @@ func TestSetup_New_MetricsDisabled(t *testing.T) {
func TestToPluginConfig_Success(t *testing.T) {
tests := []struct {
name string
cfg *telemetry.Config
cfg *Config
expectedID string
expectedConfig map[string]string
}{
{
name: "Valid config with all fields",
cfg: &telemetry.Config{
cfg: &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: true,
@@ -187,7 +186,7 @@ func TestToPluginConfig_Success(t *testing.T) {
},
{
name: "Config with enableMetrics false",
cfg: &telemetry.Config{
cfg: &Config{
ServiceName: "my-service",
ServiceVersion: "2.0.0",
EnableMetrics: false,
@@ -203,7 +202,7 @@ func TestToPluginConfig_Success(t *testing.T) {
},
{
name: "Config with empty fields",
cfg: &telemetry.Config{
cfg: &Config{
ServiceName: "",
ServiceVersion: "",
EnableMetrics: true,
@@ -259,7 +258,7 @@ func TestToPluginConfig_BooleanConversion(t *testing.T) {
for _, tt := range tests {
t.Run(tt.name, func(t *testing.T) {
cfg := &telemetry.Config{
cfg := &Config{
ServiceName: "test",
ServiceVersion: "1.0.0",
EnableMetrics: tt.enableMetrics,

View File

@@ -10,12 +10,7 @@ import (
func TestNewProviderAndMetrics(t *testing.T) {
ctx := context.Background()
provider, err := NewProvider(ctx, &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: true,
Environment: "test",
})
provider, err := NewTestProvider(ctx)
require.NoError(t, err)
require.NotNil(t, provider)
require.NotNil(t, provider.MetricsHandler)

View File

@@ -19,12 +19,7 @@ func (s stubStep) Run(ctx *model.StepContext) error {
func TestInstrumentedStepSuccess(t *testing.T) {
ctx := context.Background()
provider, err := NewProvider(ctx, &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: true,
Environment: "test",
})
provider, err := NewTestProvider(ctx)
require.NoError(t, err)
defer provider.Shutdown(context.Background())
@@ -40,12 +35,7 @@ func TestInstrumentedStepSuccess(t *testing.T) {
func TestInstrumentedStepError(t *testing.T) {
ctx := context.Background()
provider, err := NewProvider(ctx, &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: true,
Environment: "test",
})
provider, err := NewTestProvider(ctx)
require.NoError(t, err)
defer provider.Shutdown(context.Background())

View File

@@ -15,12 +15,7 @@ func TestGetStepMetrics_Success(t *testing.T) {
ctx := context.Background()
// Initialize telemetry provider first
provider, err := NewProvider(ctx, &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: true,
Environment: "test",
})
provider, err := NewTestProvider(ctx)
require.NoError(t, err)
defer provider.Shutdown(context.Background())
@@ -39,12 +34,7 @@ func TestGetStepMetrics_ConcurrentAccess(t *testing.T) {
ctx := context.Background()
// Initialize telemetry provider first
provider, err := NewProvider(ctx, &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: true,
Environment: "test",
})
provider, err := NewTestProvider(ctx)
require.NoError(t, err)
defer provider.Shutdown(context.Background())
@@ -81,12 +71,7 @@ func TestStepMetrics_Instruments(t *testing.T) {
ctx := context.Background()
// Initialize telemetry provider
provider, err := NewProvider(ctx, &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: true,
Environment: "test",
})
provider, err := NewTestProvider(ctx)
require.NoError(t, err)
defer provider.Shutdown(context.Background())
@@ -128,12 +113,7 @@ func TestStepMetrics_MultipleCalls(t *testing.T) {
ctx := context.Background()
// Initialize telemetry provider
provider, err := NewProvider(ctx, &Config{
ServiceName: "test-service",
ServiceVersion: "1.0.0",
EnableMetrics: true,
Environment: "test",
})
provider, err := NewTestProvider(ctx)
require.NoError(t, err)
defer provider.Shutdown(context.Background())

View File

@@ -2,109 +2,14 @@ package telemetry
import (
"context"
"fmt"
"net/http"
clientprom "github.com/prometheus/client_golang/prometheus"
clientpromhttp "github.com/prometheus/client_golang/prometheus/promhttp"
"go.opentelemetry.io/contrib/instrumentation/runtime"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
otelprom "go.opentelemetry.io/otel/exporters/prometheus"
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/resource"
"github.com/beckn-one/beckn-onix/pkg/log"
)
// Config represents OpenTelemetry related configuration.
type Config struct {
ServiceName string `yaml:"serviceName"`
ServiceVersion string `yaml:"serviceVersion"`
EnableMetrics bool `yaml:"enableMetrics"`
Environment string `yaml:"environment"`
}
// Provider holds references to telemetry components that need coordinated shutdown.
type Provider struct {
MeterProvider *metric.MeterProvider
MetricsHandler http.Handler
Shutdown func(context.Context) error
}
// DefaultConfig returns sensible defaults for telemetry configuration.
func DefaultConfig() *Config {
return &Config{
ServiceName: "beckn-onix",
ServiceVersion: "dev",
EnableMetrics: true,
Environment: "development",
}
}
// NewProvider wires OpenTelemetry with a Prometheus exporter and exposes /metrics handler.
func NewProvider(ctx context.Context, cfg *Config) (*Provider, error) {
if cfg == nil {
cfg = DefaultConfig()
}
if cfg.ServiceName == "" {
cfg.ServiceName = DefaultConfig().ServiceName
}
if cfg.ServiceVersion == "" {
cfg.ServiceVersion = DefaultConfig().ServiceVersion
}
if cfg.Environment == "" {
cfg.Environment = DefaultConfig().Environment
}
if !cfg.EnableMetrics {
log.Info(ctx, "OpenTelemetry metrics disabled")
return &Provider{
Shutdown: func(context.Context) error { return nil },
}, nil
}
res, err := resource.New(
ctx,
resource.WithAttributes(
attribute.String("service.name", cfg.ServiceName),
attribute.String("service.version", cfg.ServiceVersion),
attribute.String("deployment.environment", cfg.Environment),
),
)
if err != nil {
return nil, fmt.Errorf("failed to create telemetry resource: %w", err)
}
registry := clientprom.NewRegistry()
exporter, err := otelprom.New(
otelprom.WithRegisterer(registry),
otelprom.WithoutUnits(),
otelprom.WithoutScopeInfo(),
)
if err != nil {
return nil, fmt.Errorf("failed to create prometheus exporter: %w", err)
}
meterProvider := metric.NewMeterProvider(
metric.WithReader(exporter),
metric.WithResource(res),
)
otel.SetMeterProvider(meterProvider)
log.Infof(ctx, "OpenTelemetry metrics initialized for service=%s version=%s env=%s",
cfg.ServiceName, cfg.ServiceVersion, cfg.Environment)
if err := runtime.Start(runtime.WithMinimumReadMemStatsInterval(0)); err != nil {
log.Warnf(ctx, "Failed to start Go runtime instrumentation: %v", err)
}
return &Provider{
MeterProvider: meterProvider,
MetricsHandler: clientpromhttp.HandlerFor(registry, clientpromhttp.HandlerOpts{}),
Shutdown: func(ctx context.Context) error {
return meterProvider.Shutdown(ctx)
},
}, nil
}

View File

@@ -0,0 +1,54 @@
package telemetry
import (
"context"
clientprom "github.com/prometheus/client_golang/prometheus"
clientpromhttp "github.com/prometheus/client_golang/prometheus/promhttp"
"go.opentelemetry.io/otel"
"go.opentelemetry.io/otel/attribute"
otelprom "go.opentelemetry.io/otel/exporters/prometheus"
"go.opentelemetry.io/otel/sdk/metric"
"go.opentelemetry.io/otel/sdk/resource"
)
// NewTestProvider creates a minimal telemetry provider for testing purposes.
// This avoids import cycles by not depending on the otelsetup package.
func NewTestProvider(ctx context.Context) (*Provider, error) {
res, err := resource.New(
ctx,
resource.WithAttributes(
attribute.String("service.name", "test-service"),
attribute.String("service.version", "test"),
attribute.String("deployment.environment", "test"),
),
)
if err != nil {
return nil, err
}
registry := clientprom.NewRegistry()
exporter, err := otelprom.New(
otelprom.WithRegisterer(registry),
otelprom.WithoutUnits(),
otelprom.WithoutScopeInfo(),
)
if err != nil {
return nil, err
}
meterProvider := metric.NewMeterProvider(
metric.WithReader(exporter),
metric.WithResource(res),
)
otel.SetMeterProvider(meterProvider)
return &Provider{
MeterProvider: meterProvider,
MetricsHandler: clientpromhttp.HandlerFor(registry, clientpromhttp.HandlerOpts{}),
Shutdown: func(ctx context.Context) error {
return meterProvider.Shutdown(ctx)
},
}, nil
}