[pkg/stanza] Container parser not playing well with downstream operator that experiences an error

[TylerHelmuth]

Component(s)

pkg/stanza

What happened?

Description

I'm seeing strange behavior when I use the container parser chained with a regex parser that sometimes fails (because the regex is not always present in the log). When the regex_parser fails, logs collection stops. I believe this is because the container parser currently breaks out of the crioConsumer goroutine if the Write returns an error:

opentelemetry-collector-contrib/pkg/stanza/operator/parser/container/parser.go

Lines 156 to 169 in d808572

	func (p *Parser) crioConsumer(ctx context.Context) {
	entriesChan := p.crioLogEmitter.OutChannel()
	p.criConsumers.Add(1)
	defer p.criConsumers.Done()
	for entries := range entriesChan {
	for _, e := range entries {
	err := p.Write(ctx, e)
	if err != nil {
	p.Logger().Error("failed to write entry", zap.Error(err))
	return
	}
	}
	}
	}

Steps to Reproduce

Use a filelogreceiver config such as

filelog:
        exclude: []
        include:
        - /var/log/pods/*/*/*.log
        include_file_name: false
        include_file_path: true
        operators:
        - id: container-parser
          max_log_size: 102400
          type: container
        - type: regex_parser
          regexp: '\slevel=(?P<severity_field>\w+)\s'
          on_error: send_quiet
          severity:
            parse_from: attributes.severity_field
        retry_on_failure:
          enabled: true
        start_at: end

Send through a log that does not match the regex parser and then send through a log that does match.

Expected Result

Both logs are sent, with the second having a severity set.

Actual Result

Neither logs are sent.

Collector version

v0.105.0

Environment information

No response

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response

[github-actions]

Pinging code owners for pkg/stanza: @djaglowski. See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

Pinging code owners for receiver/filelog: @djaglowski. See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

Pinging code owners:

• pkg/stanza: @djaglowski

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[TylerHelmuth]

Is the solution to remove that return statement?

[ChrsMark]

Yeap, I just verified that. After the first error the "failed" log is exported but then because of this return no other logs are consumed. That's definitely problematic. Removing the mentioned return statement sounds correct and seems to solve the issue.

This return was recently added and it seems a bit weird to have the processing/parsing to stop/hang if one downstream has "failed". That doesn't look like the intended behavior for a filelog's pipeline.

This makes me think of d31bc2e#diff-1818c0cdb6108c178ca89cf5ee768fba0a0de451e9b6431464429d6bf0d0a823R60 as well. @djaglowski do you think it's correct to return immediately if a downstream operator has failed 🤔 ? If the purpose of #33847 was to ensure that errors are logged (based on #33783), then we should only log and continue without returning.

[djaglowski]

@ChrsMark I agree with your assessment. I think the loops were overlooked in both cases.

[ChrsMark]

I can prepare a patch for these and look around for any other place that might hit something similar.
I will try to cover these with tests as well.