ordering_criteria does not work for filelog receiver

[zekai-rai]

Component(s)

receiver/filelog

What happened?

Description

After setting ordering_criteria with mtime, I observed that files are always processed in the descending order of the mtime, regardless whether ascending is true or false.

I need this for both filelog receiver and otlpjsonfile receiver and tested both.

Steps to Reproduce

Configure the receiver this way, and add file one by one so that they have different last modified timestamps.

otlpjsonfile/metrics:
    include: [ /mount/shared-events/test_metrics/data_*.json ]
    start_at: "beginning" # set to "end" if delete_after_read is set to false
    poll_interval: 5s
    fingerprint_size: 5kb
    delete_after_read: true # If true, each log file will be read and then immediately deleted
    #storage: file_storage
    ordering_criteria:
      sort_by: 
        sort_type: mtime
        ascending: true

Expected Result

With sort_type: mtime and ascending: true, I expected files to be processed in the ascending order of the modified time.

Actual Result

Files are always processed in the descending order.

Collector version

v0.98

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

OpenTelemetry Collector configuration

otlpjsonfile/metrics:
    include: [ /mount/shared-events/test_metrics/data_*.json ]
    start_at: "beginning" 
    poll_interval: 5s
    fingerprint_size: 5kb
    delete_after_read: true # If true, each log file will be read and then immediately deleted
    #storage: file_storage
    ordering_criteria:
      sort_by: 
        sort_type: mtime
        ascending: true

Log output

No response

Additional context

No response

[github-actions]

Pinging code owners:

• receiver/filelog: @djaglowski

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

[JaredTan95]

Can you post sample data for data *.json?

[zekai-rai]

Can you post sample data for data *.json?

The file is large, I can probably delete some content and paste here. What are you looking for? I will make sure to keep the information you are looking for.

[crobert-1]

Hello @zekai-rai, it looks like you need to enable a feature gate for this to work properly, it's named filelog.mtimeSortType. Here's the implementation, and here's the feature gate in code.

Sorry for the confusion here, this should be documented more clearly 👍

[BinaryFissionGames]

The mtime sort option doesn't respect the ascending option currently, it looks like it's been configured correctly.

(if you try and configure sort by mtime and don't have the feature gate enabled, the collector will not start

opentelemetry-collector-contrib/pkg/stanza/fileconsumer/matcher/matcher.go

Lines 131 to 133 in f320b5f

	if !mtimeSortTypeFeatureGate.IsEnabled() {
	return nil, fmt.Errorf("the %q feature gate must be enabled to use %q sort type", mtimeSortTypeFeatureGate.ID(), sortTypeMtime)
	}

)

I think when I wrote the mtime sorting I didn't consider that you'd ever want to do an ascending sort, but when paired with the delete_on_read functionality, it could totally make sense to do, depending on your set up.

I also agree the docs could be more clear with regards to the feature gate.

[zekai-rai]

The document isn't that unclear regarding the feature gate. Without the feature gate, it gave me an error explaining the setting, so I was able to run it.

@BinaryFissionGames, is descending order because newer files are assumed more important? Our use case here is that our downstream requires submitting records in order. Is this something you would consider adding?

Btw, a basic educational question, does otlpjsonfile receiver inherit this receiver? These setting are not documented there but are available.

[BinaryFissionGames]

The descending order was just due to the main use-case being considered, which you can read in the original issue for it (it was mainly performance motivated)
#27812

I'd like to ask for @djaglowski 's opinion, but I think it's reasonable to support an ascending mtime sort.

[djaglowski]

I think the use case for ascending makes sense, given that files are being deleted as they are consumed.

[zekai-rai]

@djaglowski and @BinaryFissionGames , thank you for adding this. Much appreciated.