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

[WIP] Add support for loadbalancing with any resource attribute #18769

Conversation

zjanc
Copy link

@zjanc zjanc commented Feb 20, 2023

Description:

This PR adds support for load balancing by any resource attribute (currently the loadbalancing exporter only supports load balancing by trace ID or service name).

Changes

  • Add support for load balancing by resource attribute values
  • Fix [loadbalancingexporter] Not properly batching service traces #13826
    • consumeTrace is expected to receive ptrace.Trace with a single Resource span (i.e. behavior before service name load balancing change)
  • Add support for multiple routing keys (with fallback to traceId balancing)

Sample Configuration

loadbalancing/6:
  protocol:
    otlp:
  resolver:
    dns:
      hostname: service-1
      port: 55690
  routing_key: resource
  resource_keys: ["resource.attribute", "service.name"]

Trace ID Load Balancing Sequence Diagram

sequenceDiagram
  box trace_exporter.go
    participant ConsumeTraces
    participant consumeTracesById
    participant SplitTraces
    participant routeByTraceId

    participant consumeTrace
  end
  participant Trace Sink

  activate ConsumeTraces
  ConsumeTraces ->>+ consumeTracesById: function is chosen on exporter initialization

  consumeTracesById ->>+ SplitTraces: Sends a single ptrace.Traces
  SplitTraces ->>- consumeTracesById: Splits per RS/ILS/TraceID, returns a []ptrace.Traces
  loop For each Trace in []ptrace.Traces
    consumeTracesById ->>+ routeByTraceId: Extract the trace Id for the given batch
    routeByTraceId ->>- consumeTracesById: Returns the trace Id as a string
    consumeTracesById ->>+ consumeTrace: Send trace along with a routing key (trace Id)
    consumeTrace -)+ Trace Sink: Exports trace to endpoint based on key
    consumeTrace ->>- consumeTracesById: 
  end
  consumeTracesById ->>- ConsumeTraces: 
Loading

Resource Attribute Load Balancing Sequence Diagram

sequenceDiagram
  box trace_exporter.go
    participant ConsumeTraces
    participant consumeTracesByResource
    participant consumeTracesById
    participant splitTracesByResourceAttr

    participant consumeTrace
  end
  participant Trace Sink

  activate ConsumeTraces
  ConsumeTraces ->>+ consumeTracesByResource: function is chosen on exporter initialization

  consumeTracesByResource ->>+ splitTracesByResourceAttr: Sends a single ptrace.Traces
  Note over consumeTracesByResource, splitTracesByResourceAttr: routingEntry:<br/>routingKey routingKey<br/>keyValue string<br/>trace ptrace.Traces
  splitTracesByResourceAttr ->>- consumeTracesByResource: Splits per RS/Resource Attribute, returns a []routingEntry
  loop For each in []routingEntry
    alt routingKey == resourceAttrRouting
      Note over consumeTracesByResource, consumeTrace: if any of the resource attributes were found in the  trace<br/>we send directly to consumeTrace
      consumeTracesByResource ->>+ consumeTrace: Send trace along with a routing key (resource attribute value)
    else routingKey == traceIDRouting
      Note over consumeTracesByResource, consumeTrace: if NONE of the resource attributes were found <br/>we fall back to Trace ID balancing
      consumeTracesByResource ->>+ consumeTracesById: We pass the trace to be handled by Id balancing consumer
      consumeTracesById ->> consumeTrace: Follow the process in the Trace ID Balancing Diagram
    end
    consumeTrace -)+ Trace Sink: Exports trace to endpoint based on key
    consumeTrace ->>- consumeTracesByResource: 
  end
  consumeTracesByResource ->>- ConsumeTraces: 
Loading

To-do

  • Improve test coverage
  • Update README

Link to tracking Issue:

  • Also fixes related issue:

@linux-foundation-easycla
Copy link

linux-foundation-easycla bot commented Feb 20, 2023

CLA Signed

The committers listed above are authorized under a signed CLA.

Copy link
Member

@jpkrohling jpkrohling left a comment

Choose a reason for hiding this comment

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

I like the feature, but I think it could be done so that each routing key has its router. Trying to share code among them is likely to cause performance regressions, and I wouldn't be comfortable accepting this change as is without evidence that the performance is at least on par with the current implementation.

exporter/loadbalancingexporter/config.go Outdated Show resolved Hide resolved
@@ -54,7 +55,7 @@ func createDefaultConfig() component.Config {
}

func createTracesExporter(_ context.Context, params exporter.CreateSettings, cfg component.Config) (exporter.Traces, error) {
return newTracesExporter(params, cfg)
return newTracesExporter(params, cfg, zap.NewNop())
Copy link
Member

Choose a reason for hiding this comment

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

You can actually get a logger from params.

exporter/loadbalancingexporter/testdata/config.yaml Outdated Show resolved Hide resolved
exporter/loadbalancingexporter/testdata/config.yaml Outdated Show resolved Hide resolved
@@ -73,6 +80,60 @@ func buildExporterConfig(cfg *Config, endpoint string) otlpexporter.Config {
return oCfg
}

func SplitTracesByResourceAttr(batches ptrace.Traces, attrKeys []string) (map[string][]ptrace.Traces, error) {
// This code is based on the ConsumeTraces function of the batchperresourceattr
Copy link
Member

Choose a reason for hiding this comment

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

Shouldn't this be part of a common library then between the two components?

exporter/loadbalancingexporter/trace_exporter.go Outdated Show resolved Hide resolved
exporter/loadbalancingexporter/trace_exporter.go Outdated Show resolved Hide resolved
@jpkrohling
Copy link
Member

CLA check seems to be failing. Once this is fixed, I'll do another review.

@github-actions
Copy link
Contributor

This PR was marked stale due to lack of activity. It will be closed in 14 days.

@github-actions github-actions bot added the Stale label May 26, 2023
@github-actions
Copy link
Contributor

Closed as inactive. Feel free to reopen if this PR is still being worked on.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[loadbalancingexporter] Not properly batching service traces
2 participants