[pkg/stanza] Add monitoring metrics for open and harvested files in fileconsumer #31544

ChrsMark · 2024-03-04T10:53:31Z

Blocked on #31618

Description:

This PR adds support for filelog receiver to emit observable metrics about its current state: how many files are opened, and harvested.

Link to tracking Issue: #31256

Testing:

How to test this manually

Use the following collector config:

receivers:
  filelog:
    start_at: end
    include:
    - /var/log/busybox/monitoring/*.log
exporters:
  debug:
    verbosity: detailed
service:
  telemetry:
    metrics:
      level: detailed
      address: ":8888"
  pipelines:
    logs:
      receivers: [filelog]
      exporters: [debug]
      processors: []

Build and run the collector: make otelcontribcol && ./bin/otelcontribcol_linux_amd64 --config ~/otelcol/monitoring_telemetry/config.yaml
Produce some logs:

echo 'some line' >> /var/log/busybox/monitoring/1.log
while true; do echo -e "This is a log line" >> /var/log/busybox/monitoring/2.log; done

Verify that metrics are produced:

curl 0.0.0.0:8888/metrics | grep _files
  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100  4002    0  4002    0     0  1954k      0 --:--:-- --:--:-- --:--:-- 1954k
# HELP otelcol_fileconsumer_open_files Number of open files
# TYPE otelcol_fileconsumer_open_files gauge
otelcol_fileconsumer_open_files{service_instance_id="72b4899d-6ce3-41de-a25b-8f0370e22ec1",service_name="otelcontribcol",service_version="0.99.0-dev"} 2
# HELP otelcol_fileconsumer_reading_files Number of open files that are being read
# TYPE otelcol_fileconsumer_reading_files gauge
otelcol_fileconsumer_reading_files{service_instance_id="72b4899d-6ce3-41de-a25b-8f0370e22ec1",service_name="otelcontribcol",service_version="0.99.0-dev"} 1

Documentation:
Added a respective section in Filelog receiver's docs.

djaglowski

Do you mind splitting this into two PRs - one to pass in component.TelemetrySettings, the other to add the metrics?

pkg/stanza/operator/input/tcp/tcp_test.go

pkg/stanza/fileconsumer/file.go

pkg/stanza/fileconsumer/config.go

pkg/stanza/fileconsumer/file.go

github-actions · 2024-03-21T05:19:40Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

ChrsMark · 2024-03-21T11:39:41Z

Still WiP.

github-actions · 2024-04-06T05:19:22Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

ChrsMark · 2024-04-06T10:27:20Z

Still WiP

github-actions · 2024-04-22T05:19:30Z

This PR was marked stale due to lack of activity. It will be closed in 14 days.

djaglowski · 2024-05-17T14:05:08Z

pkg/stanza/fileconsumer/config.go

+	meter := set.MeterProvider.Meter("otelcol/fileconsumer")
+
+	openedFiles, err := meter.Int64UpDownCounter(
+		"fileconsumer"+"/"+"open_files",


Was this broken up because there was an intention to use constants?

Not really, but good point. I will make those constants!

djaglowski · 2024-05-17T14:20:00Z

pkg/stanza/fileconsumer/file.go

@@ -105,6 +110,7 @@ func (m *Manager) startPoller(ctx context.Context) {

 // poll checks all the watched paths for new entries
 func (m *Manager) poll(ctx context.Context) {
+	m.reportMetrics(ctx)


Would it be possible to increment whenever we open a file and decrement whenever we close one?

I guess that would doable inside the Tracker. Specifically inside the Add(reader *reader.Reader) and ClosePreviousFiles() methods. Is that what you had in mind?

In that case, the metric should be called otelcol_tracker_open_files right?

I think the name can stay the same. IMO users don't need to be exposed to the internal package structure. Sticking with fileconsumer seems specific enough.

Would it be possible to increment whenever we open a file and decrement whenever we close one?

So, files are opened by the Manager but are closed by the Manager and the Tracker as well from what I can see.
Also inside the Manager's operations there are multiple places where files are re-opened and then their readers are closed again like at https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.100.0/pkg/stanza/fileconsumer/file.go#L203 or https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.100.0/pkg/stanza/fileconsumer/file.go#L222.

Based on this I thought that it makes sense to simplify and only increment the openFiles counter only when we create a fresh reader at https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/v0.100.0/pkg/stanza/fileconsumer/file.go#L225-L232.

Incrementing in previous steps and then possibly decrementing after a couple of instrunctions didn't make a lot of sense to me but let me know what you think :).

newly12 · 2024-05-21T01:42:19Z

Hi @ChrsMark, one quick question, are the new metrics shown at detailed metrics level or normal level?

ChrsMark · 2024-05-21T07:15:50Z

Hi @ChrsMark, one quick question, are the new metrics shown at detailed metrics level or normal level?

They are shown on the normal level as well.

djaglowski · 2024-05-21T13:34:04Z

pkg/stanza/fileconsumer/internal/tracker/tracker.go

@@ -106,6 +131,7 @@ func (t *fileTracker) ClosePreviousFiles() {

 	for r, _ := t.previousPollFiles.Pop(); r != nil; r, _ = t.previousPollFiles.Pop() {
 		t.knownFiles[0].Add(r.Close())
+		t.openFiles.Add(context.TODO(), -1)


This is really the only opening or closing of files we do in the tracker, so it does feel quite awkward to have the tracker manage the meter. Instead, what if we just return the number of files closed from this method? We're closing them one after another, so decrementing after each one or all at once seems equivalent in this case.

In that case we would need to either have the EndConsume() to return the number of files closed as well. ~~or to move the ClosePreviousFiles() outside of it. I'm leaning towards the latter.~~

It seems that the calling order of the ClosePreviousFiles() is different on Windows so we cannot decouple it from the EndConsume()

The latter won't work because the order of operations is different depending on OS.

I think we could get rid of OS dependent behavior for closing files if we implement #32037.

Otherwise, I'm fine with returning the count from EndConsume.

I had missed that EndConsume of noStateTracker closes files directly so we need to have it returning the number of files closed anyway.

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

djaglowski · 2024-05-22T18:23:32Z

pkg/stanza/fileconsumer/file.go

@@ -73,7 +77,7 @@ func (m *Manager) Stop() error {
 		m.cancel = nil
 	}
 	m.wg.Wait()
-	m.tracker.ClosePreviousFiles()
+	m.openFiles.Add(context.TODO(), int64(0-m.tracker.ClosePreviousFiles()))


Are you willing to look into what it takes to pass the context through?

If I'm not mistaken we could pass down the ctx from the Shutdown of the stanza receiver when we stop the internal pipeline at

opentelemetry-collector-contrib/pkg/stanza/adapter/receiver.go

Line 142 in d78d7bb

pipelineErr := r.pipe.Stop()

.

Then if we pass the ctx on every single operator's Stop() we can make it available to the Stop() of the Manager/fileconsumer at

opentelemetry-collector-contrib/pkg/stanza/operator/input/file/input.go

Line 36 in d78d7bb

return i.fileConsumer.Stop()

when we stop the file_input operator.

This would be doable I think but would be a breaking change for the Pipeline interface and the Operator interface (plus the Manager).
Hence, I'm not sure if that's worth doing it unless we find value in doing so anyways and not just for the meter's need?

github-actions bot added pkg/stanza receiver/otlpjsonfile labels Mar 4, 2024

github-actions bot requested review from atoulme and djaglowski March 4, 2024 10:54

ChrsMark force-pushed the add_filelog_metrics branch 2 times, most recently from 90e25c4 to 013941a Compare March 4, 2024 13:13

github-actions bot added the processor/logstransform Logs Transform processor label Mar 4, 2024

github-actions bot requested a review from dehaansa March 4, 2024 13:14

ChrsMark force-pushed the add_filelog_metrics branch 6 times, most recently from a51c88b to bb8d0f1 Compare March 5, 2024 12:59

djaglowski reviewed Mar 5, 2024

View reviewed changes

pkg/stanza/operator/input/tcp/tcp_test.go Outdated Show resolved Hide resolved

pkg/stanza/fileconsumer/file.go Show resolved Hide resolved

pkg/stanza/fileconsumer/config.go Outdated Show resolved Hide resolved

pkg/stanza/fileconsumer/file.go Outdated Show resolved Hide resolved

ChrsMark mentioned this pull request Mar 6, 2024

[receiver/filelog] Add support for telemetry metrics #31618

Closed

ChrsMark changed the title ~~[receiver/filelog ] Add monitoring metrics about open and harvested files~~ [receiver/filelog] Add monitoring metrics about open and harvested files Mar 6, 2024

ChrsMark force-pushed the add_filelog_metrics branch 3 times, most recently from df7ecbd to 00e930a Compare March 6, 2024 11:57

github-actions bot added the Stale label Mar 21, 2024

github-actions bot removed the Stale label Mar 22, 2024

github-actions bot added the Stale label Apr 6, 2024

github-actions bot removed the Stale label Apr 7, 2024

github-actions bot added the Stale label Apr 22, 2024

ChrsMark force-pushed the add_filelog_metrics branch from 6927a60 to ade99b7 Compare May 17, 2024 10:42

ChrsMark changed the title ~~[receiver/filelog] Add monitoring metrics about open and harvested files~~ [pkg/stanza] Add monitoring metrics for open and harvested files in fileconsumer May 17, 2024

ChrsMark force-pushed the add_filelog_metrics branch from ade99b7 to 6059fe4 Compare May 17, 2024 11:08

ChrsMark requested a review from djaglowski May 17, 2024 11:31

djaglowski reviewed May 17, 2024

View reviewed changes

ChrsMark force-pushed the add_filelog_metrics branch 6 times, most recently from 8010c71 to cdff3b2 Compare May 20, 2024 14:42

ChrsMark force-pushed the add_filelog_metrics branch from cdff3b2 to e576ec9 Compare May 21, 2024 07:27

djaglowski reviewed May 21, 2024

View reviewed changes

ChrsMark force-pushed the add_filelog_metrics branch 3 times, most recently from 32f8e54 to 52aeef2 Compare May 21, 2024 14:29

[receiver/filelog] Add monitoring metrics about open and harvested files

4d5b80b

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

ChrsMark force-pushed the add_filelog_metrics branch 3 times, most recently from 35d922f to a94b94f Compare May 22, 2024 09:11

increment/decrement openfiles metric on file open/close

cff6b64

Signed-off-by: ChrsMark <chrismarkou92@gmail.com>

ChrsMark force-pushed the add_filelog_metrics branch from a94b94f to cff6b64 Compare May 22, 2024 09:21

ChrsMark requested a review from djaglowski May 22, 2024 09:23

djaglowski approved these changes May 22, 2024

View reviewed changes

djaglowski merged commit 4ba2c7c into open-telemetry:main May 22, 2024
162 checks passed

github-actions bot added this to the next release milestone May 22, 2024

djaglowski reviewed May 22, 2024

View reviewed changes

ChrsMark mentioned this pull request May 23, 2024

[receiver/filelog] Improve receiver logging and monitoring metrics #31256

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[pkg/stanza] Add monitoring metrics for open and harvested files in fileconsumer #31544

[pkg/stanza] Add monitoring metrics for open and harvested files in fileconsumer #31544

ChrsMark commented Mar 4, 2024 •

edited

djaglowski left a comment

github-actions bot commented Mar 21, 2024

ChrsMark commented Mar 21, 2024

github-actions bot commented Apr 6, 2024

ChrsMark commented Apr 6, 2024

github-actions bot commented Apr 22, 2024

djaglowski May 17, 2024

ChrsMark May 17, 2024

djaglowski May 17, 2024

ChrsMark May 17, 2024

djaglowski May 17, 2024

ChrsMark May 21, 2024

newly12 commented May 21, 2024 •

edited

ChrsMark commented May 21, 2024

djaglowski May 21, 2024

ChrsMark May 21, 2024 •

edited

djaglowski May 21, 2024

ChrsMark May 21, 2024

djaglowski May 22, 2024

ChrsMark May 23, 2024 •

edited

[pkg/stanza] Add monitoring metrics for open and harvested files in fileconsumer #31544

[pkg/stanza] Add monitoring metrics for open and harvested files in fileconsumer #31544

Conversation

ChrsMark commented Mar 4, 2024 • edited

How to test this manually

djaglowski left a comment

Choose a reason for hiding this comment

github-actions bot commented Mar 21, 2024

ChrsMark commented Mar 21, 2024

github-actions bot commented Apr 6, 2024

ChrsMark commented Apr 6, 2024

github-actions bot commented Apr 22, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

newly12 commented May 21, 2024 • edited

ChrsMark commented May 21, 2024

Choose a reason for hiding this comment

ChrsMark May 21, 2024 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

ChrsMark May 23, 2024 • edited

Choose a reason for hiding this comment

ChrsMark commented Mar 4, 2024 •

edited

newly12 commented May 21, 2024 •

edited

ChrsMark May 21, 2024 •

edited

ChrsMark May 23, 2024 •

edited