Skip to content

[bug] Fixed silent log drops when more than 1000 lines.#49818

Open
krshn-ptl wants to merge 2 commits into
open-telemetry:mainfrom
krshn-ptl:main
Open

[bug] Fixed silent log drops when more than 1000 lines.#49818
krshn-ptl wants to merge 2 commits into
open-telemetry:mainfrom
krshn-ptl:main

Conversation

@krshn-ptl

Copy link
Copy Markdown

Description

When extracting lines of logs with text encoding, the code ensures that logs are processed in chunks but flushed at once. The default flush values are 1000x1024x1024 bytes or 1000 items. During streaming, we override the defaults so that flushes happen at once. We override the default for bytes, but the items override was missed. This causes logs greater than 1000 lines to silently drop.
The PR adds a change to override the default items to 0 so that the flush happens for all logs once they are prepared. There were two ways to fix this:

  • The current implementation.
  • Flushing in chunks instead of holding everything in memory.

I chose the first approach because I may not have proper context on why flush-all was done in the first place.

History (How I came across this, and design choices)

I am using OTEL for extracting AWS ELB logs. As per the standard AWS setup, these logs are delivered as S3 objects that I need to onboard into my DB. For this, I use the awss3receiver with a queue in place via which notifications come for every new S3 file creation. Since these are compressed objects, we need extensions to extract and process each line of the log separately.

I am using textencoding instead of the awslogencoding extension because that has two major blockers for me:

  • The body is not propagated further after the receiver layer.
  • Not all data is extracted (for example, received_at time is not extracted).

By using text encoding, I wrote a transformer over it to mimic the extraction layer at my config end. This helps me with any type of customization as well.

While doing this, I noticed logs were getting dropped. After debugging, I found this bug.

Testing

I've added a test to verify that if the streamed log is more than 1000, then it shouldn't drop it, and all logs are to be flushed. (AI-generated test case)

Authorship

  • I, a human, wrote this pull request description myself.

@krshn-ptl
krshn-ptl requested review from a team, MovieStoreGuy and atoulme as code owners July 22, 2026 05:57
@github-actions github-actions Bot added the first-time contributor PRs made by new contributors label Jul 22, 2026
@github-actions

Copy link
Copy Markdown
Contributor

Welcome, contributor! Thank you for your contribution to opentelemetry-collector-contrib.

Important reminders:

  • Read our Contributing Guidelines.
  • Sign the CLA if you haven't already.
  • Follow the OpenTelemetry Generative AI policy: disclose any AI use in your contribution, and communicate (PR descriptions, review replies) in your own words rather than AI-generated text.
  • Give reviewers at least a few days before pinging them for feedback.
  • If you need help with general setup, development process, or contributor etiquette, ask in #opentelemetry-new-contributors.
  • First-time contributors should have at most one PR not marked as draft until their first PR is merged.
  • If your change isn't one of our priority components, reviews may take more time.
  • Raise technical or Collector-specific questions in #otel-collector-dev or a Collector SIG meeting.

@opentelemetry-pr-dashboard

opentelemetry-pr-dashboard Bot commented Jul 22, 2026

Copy link
Copy Markdown

Pull request dashboard status

Status last refreshed: 2026-07-23 00:30:14 UTC.

  • Waiting on: Author
  • Next steps:
    • Investigate required status check failures.
    • Address or respond to 1 review feedback item:
      • Top-level feedback: 1
      • For each item, reply to move the discussion forward, e.g. link to the commit that addresses it, explain why no change is needed, or ask a follow-up question.

This automated status or its linked feedback items may be incorrect. If something looks wrong, please report it with the result you expected. If you believe this pull request is incorrectly routed as waiting on the author, comment /dashboard route:reviewers to route it from waiting on the author to waiting on reviewers. If the last refreshed time above predates your latest reply or push, the dashboard hasn't processed it yet.

@paulojmdias

Copy link
Copy Markdown
Member

/workflow-approve

Comment thread .chloggen/main.yaml Outdated

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Pull request overview

Fixes a bug in the textencoding extension where UnmarshalLogs could silently return only the first decoded batch when inputs exceed the default stream-decoder flush item threshold (1000), leading to dropped log records.

Changes:

  • Disable item-count based flushing in UnmarshalLogs by adding encoding.WithFlushItems(0) alongside WithFlushBytes(0).
  • Add a regression test that verifies UnmarshalLogs returns all records when input contains >1000 log lines.
  • Add a changelog entry documenting the bug fix.

Reviewed changes

Copilot reviewed 3 out of 3 changed files in this pull request and generated 1 comment.

File Description
extension/encoding/textencodingextension/text.go Ensures UnmarshalLogs does not flush early on item count, preventing truncation for large inputs.
extension/encoding/textencodingextension/text_test.go Adds a regression test covering >1000-line inputs to prevent future silent truncation regressions.
.chloggen/main.yaml Adds release note entry for the bug fix (needs filename convention alignment).
Comments suppressed due to low confidence (1)

extension/encoding/textencodingextension/text_test.go:13

  • The import block is not gofmt-compliant (std imports are out of order: "strings" appears before "io"). This will typically fail gofmt/lint checks in CI.
import (
	"bytes"
	"fmt"
	"strings"
	"io"
	"regexp"
	"testing"


💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Comment thread .chloggen/main.yaml Outdated
Comment on lines +1 to +7
# Use this changelog template to create an entry for release notes.

# One of 'breaking', 'deprecation', 'new_component', 'enhancement', 'bug_fix'
change_type: bug_fix

# The name of the component, or a single word describing the area of concern, (e.g. receiver/filelog)
component: extension/encoding

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

Not sure about this comment. Please confirm if such change is required

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Not needed 👍

Co-authored-by: Paulo Dias <44772900+paulojmdias@users.noreply.github.com>
@paulojmdias

Copy link
Copy Markdown
Member

@krshn-ptl can you please look into CI errors?

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.

4 participants