Skip to content

[chore] [exporter/splunk_hec] Remove dead code#38113

Merged
dmitryax merged 1 commit into
open-telemetry:mainfrom
dmitryax:hec-exporter-remove-dead-code
Feb 22, 2025
Merged

[chore] [exporter/splunk_hec] Remove dead code#38113
dmitryax merged 1 commit into
open-telemetry:mainfrom
dmitryax:hec-exporter-remove-dead-code

Conversation

@dmitryax
Copy link
Copy Markdown
Member

@dmitryax dmitryax commented Feb 22, 2025

splunk.HandleHTTPCode always returns an error for any codes other than 2xx. So, the removed condition is always true. The body isn't read for http.StatusTooManyRequests and http.StatusServiceUnavailable error codes anyway. It's only read for http.StatusBadRequest, http.StatusUnauthorized and explicitly drained for 2xx.

I kept the existing behavior of returning the draining errors, but this is wrong. Response body draining errors should not be considered data delivery failures which trigger the retry mechanism by default. I'll fix it in a follow-up PR.

Another fix to do as a follow-up is to ensure the body is drained for all error codes except for http.StatusTooManyRequests and http.StatusServiceUnavailable.

`splunk.HandleHTTPCode` always returns an error for any codes other than 2xx. So the removed condition is always true and we always drain the response body
@dmitryax dmitryax force-pushed the hec-exporter-remove-dead-code branch from 2b59033 to cf7b923 Compare February 22, 2025 06:05
@dmitryax dmitryax merged commit db59ed2 into open-telemetry:main Feb 22, 2025
@github-actions github-actions Bot added this to the next release milestone Feb 22, 2025
@dmitryax dmitryax deleted the hec-exporter-remove-dead-code branch February 22, 2025 18:35
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.

3 participants