open-telemetry / opentelemetry-operator

Kubernetes Operator for OpenTelemetry Collector
Apache License 2.0
1.18k stars 420 forks source link

Remove unnecessary config marshalling in v1beta1 #2603

Open pavolloffay opened 7 months ago

pavolloffay commented 7 months ago

Component(s)

collector

Describe the issue you're reporting

Created from https://github.com/open-telemetry/opentelemetry-operator/pull/2532

crossoverJie commented 5 months ago

@pavolloffay I'd like to fix this issue.

We can create a hardcoded method:

func (c *Config) ToMap() map[string]interface{} {
    result := make(map[string]interface{})

    result["receivers"] = c.Receivers.Object
    result["exporters"] = c.Exporters.Object
    if c.Processors != nil {
        result["processors"] = c.Processors.Object
    }

    if c.Connectors != nil {
        result["connectors"] = c.Connectors.Object
    }
    if c.Extensions != nil {
        result["extensions"] = c.Extensions.Object

    }
    serviceMap := make(map[string]interface{})
    if c.Service.Extensions != nil {
        serviceMap["extensions"] = c.Service.Extensions
    }
    if c.Service.Telemetry != nil {
        serviceMap["telemetry"] = c.Service.Telemetry
    }
    serviceMap["pipelines"] = c.Service.Pipelines.Object
    result["service"] = serviceMap

    return result
}

Or use serialization method:

func (c *Config) ToMap() (map[string]interface{}, error) {
    var result map[string]interface{}
    v, err := json.Marshal(c)
    if err != nil {
        return nil, err
    }
    err = json.Unmarshal(v, &result)
    if err != nil {
        return nil, err
    }
    return result, err
}

But this will still cause multiple calls to the serialization and deserialization method.

BWT, maybe we can replace map[interface{}]interface{} with map[string]interface{} so we can avoid redundant type assertions.

https://github.com/open-telemetry/opentelemetry-operator/blob/e58df33539f7b0af378b1998e6e4e107d86f00d3/internal/manifests/collector/adapters/config_validate.go#L68

jaronoff97 commented 5 months ago

@crossoverJie yep, already working on this :)