Skip to content

[exporter/stefexporter] Fix incorrectly implemented STEF exporter zstd compression option #38089

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

Merged
merged 1 commit into from
Feb 21, 2025

Conversation

tigrannajaryan
Copy link
Member

Description

Resolves #38088

Previously the compression option was incorrectly set both for STEF Writer and gRPC Client. This was unnecessary and also did not work if the destination gRPC receiver did not enable zstd compressor.

This change correctly removes the zstd compression option from gRPC client and only sets it as a STEF Writer option.

Interestingly, with zstd enabled we get less than 1 byte per metric data point with STEF exporter and hostmetrics receiver!

$ ./stefmockserver_darwin_arm64
2025/02/20 11:28:48.335835 Listening for STEF/gRPC on port 4320
2025/02/20 11:49:55.002889 Incoming STEF/gRPC connection.
Records: 1285639, Messages: 4420, Bytes: 1139887, Bytes/point: 0.89, Acks: 4424, Last ACKID: 1285639

Testing Done

Added zstd compression option to TestExport unit test.

Tested manually with STEF Server here https://github.com/splunk/stef/tree/main/otelcol/cmd/stefmockserver

Built Collector contrib via make otelcontribcol. Used the following config for testing:

receivers:
  hostmetrics:
    collection_interval: 1s
    scrapers:
      load:
      filesystem:
      memory:
      network:
      paging:
      processes:

exporters:
  debug:
  stef:
    endpoint: localhost:4320
    compressions: zstd
    tls:
      insecure: true

processors:

service:
  pipelines:
    metrics:
      receivers: [hostmetrics]
      processors: []
      exporters: [debug,stef]

@dmitryax
Copy link
Member

@tigrannajaryan please run make gotidy

### Description

Resolves open-telemetry#38088

Previously the compression option was incorrectly set both
for STEF Writer and gRPC Client. This was unnecessary and
also did not work if the destination gRPC receiver did not
enable zstd compressor.

This change correctly removes the zstd compression option
from gRPC client and only sets it as a STEF Writer option.

### Testing Done

Added zstd compression option to TestExport unit test.

Tested manually with STEF Server here https://github.com/splunk/stef/tree/main/otelcol/cmd/stefmockserver

Built Collector contrib via `make otelcontribcol`. Used the following config for testing:

```yaml
receivers:
  hostmetrics:
    collection_interval: 1s
    scrapers:
      load:
      filesystem:
      memory:
      network:
      paging:
      processes:

exporters:
  debug:
  stef:
    endpoint: localhost:4320
    compressions: zstd
    tls:
      insecure: true

processors:

service:
  pipelines:
    metrics:
      receivers: [hostmetrics]
      processors: []
      exporters: [debug,stef]
```

Interestingly, with zstd enabled we get less than 1 byte per
metric data point with STEF!

```
$ ./stefmockserver_darwin_arm64
2025/02/20 11:28:48.335835 Listening for STEF/gRPC on port 4320
2025/02/20 11:49:55.002889 Incoming STEF/gRPC connection.
Records: 1285639, Messages: 4420, Bytes: 1139887, Bytes/point: 0.89, Acks: 4424, Last ACKID: 1285639
```
@tigrannajaryan
Copy link
Member Author

@tigrannajaryan please run make gotidy

Done.

However, the build now fails on make gofmt, unclear why and it is on a different component, not touched by this PR: https://github.com/open-telemetry/opentelemetry-collector-contrib/actions/runs/13446709218/job/37573575408?pr=38089

I have run make gofmt locally and it finishes cleanly. Any thoughts on what the problem is?

@dmitryax dmitryax merged commit 98e6c42 into open-telemetry:main Feb 21, 2025
162 checks passed
@github-actions github-actions bot added this to the next release milestone Feb 21, 2025
@dmitryax
Copy link
Member

I have run make gofmt locally and it finishes cleanly. Any thoughts on what the problem is?

That looks like a flakyness

@tigrannajaryan tigrannajaryan deleted the tigran/stefcompression branch February 21, 2025 16:53
yiquanzhou added a commit to dash0hq/opentelemetry-collector-contrib that referenced this pull request Feb 24, 2025
* main: (55 commits)
  [chore] Update core dependencies (open-telemetry#38124)
  Add kafka topics observer implementation (open-telemetry#38060)
  [exporter/splunk_hec] Mute errors from draining the response body (open-telemetry#38118)
  [chore] [exporter/splunk_hec] Remove dead code (open-telemetry#38113)
  Add support for JUnit test results (open-telemetry#37941)
  [chore] amend changelog for prometheus receiver change (open-telemetry#38109)
  [chore] Fix dead links in issue-triaging.md (open-telemetry#38105)
  [chore] fix deprecation (open-telemetry#38107)
  [exporter/coralogix] Add new batch options to Coralogix exporter (open-telemetry#38082)
  [chore][exporter/datadog] fix integration test (open-telemetry#38091)
  [chore] Update otel to unblock contrib test in core repo (open-telemetry#38100)
  [chore] Bump go-version match to 1.23 (open-telemetry#38099)
  [exporter/elasticsearch] Add _metric_names_hash to avoid metric rejections (open-telemetry#37511)
  elasticsearchexporter: refactor encoding; drop metrics support from raw/none/bodymap mapping modes (open-telemetry#37928)
  [exporter/stefexporter] Fix incorrectly implemented STEF exporter zstd compression option (open-telemetry#38089)
  [exporter/clickhouse] Add client info for identifying exporter in `system.query_log` (open-telemetry#37146)
  [chore] Prepare release 0.120.1 (open-telemetry#38055)
  [extension/httpforwarder] Shutdown should wait server exit (open-telemetry#37735)
  receiver/prometheusremotewrite: Add two fields timestamp and value. (open-telemetry#37895)
  [reciver/sqlqueryreceiver] Add support for SapASE (sybase) (open-telemetry#37773)
  ...
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.

STEF exporter incorrectly double-compresses output
3 participants