[receiver/filelog] Add support for telemetry metrics #31618
Conversation
github-actions
bot
added
pkg/stanza
processor/logstransform
ChrsMark
force-pushed
the
add_telemetry_settings_to_filelog_comps
branch
8 times, most recently
from
59f6660
to
66cecfc
Compare
ChrsMark
changed the title
ChrsMark
force-pushed
the
add_telemetry_settings_to_filelog_comps
branch
from
66cecfc
to
afefc9d
Compare
Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
ChrsMark
force-pushed
the
add_telemetry_settings_to_filelog_comps
branch
from
afefc9d
to
d38caf1
Compare
| @@ -25,7 +26,7 @@ func NewConfig(b Builder) Config { | |||
| type Builder interface { | |||
| ID() string | |||
| Type() string | |||
| Build(*zap.SugaredLogger) (Operator, error) | |||
There was a problem hiding this comment.
It's unfortunate, but because this is exported, we'll have to deprecate it as is an replace it with another version of the function. This is a lot to ask so I'll take a pass at it and open a PR for it.
There was a problem hiding this comment.
Ah ok I see. Thank's for handling this. Let me know if there is anything I could help with.
There was a problem hiding this comment.
I've made some progress on this but the migrating to a new interface leads to a lot of changes throughout the codebase. I have a few more things to work through next week and then can start peeling off PRs & hopefully isolate some things which can be done in parallel.
There was a problem hiding this comment.
Unfortunately I have been spinning my wheels on the implementation I thought would work. I am offline next week so don't want to hold this up if you want to make another attempt at it. Otherwise I'll plan on revisiting when I'm back.
The exercise was not entirely pointless as some requirements, nice to haves, and implications were made clear. We need to deprecate this Build function and keep it working while we establish a new pattern. The new solution should work with component.TelemetrySettings instead of zap.SugaredLogger. This affects at a minimum:
- Every operator
- The
pipelinebuild logic - The
adapterinterface and build logic - Registration and lookup of operator types
- Unmarshaling of operator configs (specifically the representation we unmarshal them into)
The real challenge here is the deprecation process because it would be easy enough to replace directly.
Because of the wide reach of the interface, I thought it may be best to sidestep the direct replacement problem and use a different mechanism for registration, lookup, unmarshaling, and building. Basically, I tried to mirror what we have in our components by introducing a parallel-but-independent mechanism for all of this, based on patterns we use in the higher level components. This included use of component.ID, component.Config, and a factory pattern w/ GetDefaultConfig and CreateOperator. There seemed to be some promising benefit to this approach but I ultimately got stuck trying to work out the unmarshaling logic. There's just a fundamentally different pattern to unmarshaling operator configs vs component configs because of how identity is managed (inside vs outside).
At some point here we may just have to pre-announce a coming breaking change and then do a hard switch over, but we need to rule out other options first. I'm not yet ready to hard break all users of the interface so I think we need to look at other potential implementations first. If you have ideas please feel free, otherwise I'll make another (less ambitious) attempt after next week.
There was a problem hiding this comment.
Thank's for the update. I will have a look next week and share back any findings.
There was a problem hiding this comment.
I didn't manage to achieve a lot beyond verifying the issue and trying to massage #31664 to work.
Even if I managed to resolve the compiling errors, then on runtime the Collector fails to unmarshal configs
based on the new approach 😞:
Error: failed to get config: cannot unmarshal the configuration: 1 error(s) decoding:
* error decoding 'receivers': error reading configuration for "filelog": 3 error(s) decoding:
* 'operators[0]' expected type 'operator.Identifiable', got 'map[string]interface {}'
* 'operators[1]' expected type 'operator.Identifiable', got 'map[string]interface {}'
* 'operators[2]' expected type 'operator.Identifiable', got 'map[string]interface {}'
2024/04/11 15:58:24 collector server run finished with error: failed to get config: cannot unmarshal the configuration: 1 error(s) decoding:
Based on the fact that the main issue seems to be the fundamentally different pattern to unmarshaling operator configs vs component configs because of how identity is managed (inside vs outside) I wonder if we should take a step back and think of fixing this at first place?
Not sure if this is technically doable though.
|
This PR was marked stale due to lack of activity. It will be closed in 14 days. |
|
Keep it open: WiP |
|
Too many conflicts to resolve after other refactoring that took place. |
…erface (#32662) **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> This PR resumes the work from #31618. The goal is to pass in the `component.TelemetrySettings` so as to use them later in various ways: 1) report the filelog's state stats: #31544 2) switch from `SugaredLogger` to `Logger`: #32177 More about the breaking change decision at #31618 (comment). **Link to tracking Issue:** <Issue number if applicable> #31256 **Testing:** <Describe what testing was performed and which tests were added.> Testing suite got updated. #### Manual Testing 1. ```yaml receivers: filelog: start_at: end include: - /var/log/busybox/refactroring_test.log operators: - id: addon type: add field: attributes.extra value: my-val exporters: debug: verbosity: detailed service: pipelines: logs: receivers: [filelog] exporters: [debug] processors: [] ``` 2. `./bin/otelcontribcol_linux_amd64 --config ~/otelcol/config.yaml` 3. `echo 'some line' >> /var/log/busybox/refactroring_test.log` 4. ```console 2024-04-24T09:29:17.104+0300 info LogsExporter {"kind": "exporter", "data_type": "logs", "name": "debug", "resource logs": 1, "log records": 1} 2024-04-24T09:29:17.104+0300 info ResourceLog #0 Resource SchemaURL: ScopeLogs #0 ScopeLogs SchemaURL: InstrumentationScope LogRecord #0 ObservedTimestamp: 2024-04-24 06:29:17.005433317 +0000 UTC Timestamp: 1970-01-01 00:00:00 +0000 UTC SeverityText: SeverityNumber: Unspecified(0) Body: Str(some line) Attributes: -> extra: Str(my-val) -> log.file.name: Str(1.log) Trace ID: Span ID: Flags: 0 {"kind": "exporter", "data_type": "logs", "name": "debug"} ``` **Documentation:** <Describe the documentation added.> TBA. Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
…ileconsumer (#31544) Blocked on #31618 **Description:** <Describe what has changed.> <!--Ex. Fixing a bug - Describe the bug and how this fixes the issue. Ex. Adding a feature - Explain what this achieves.--> 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:** <Describe what testing was performed and which tests were added.> #### How to test this manually 1. Use the following collector config: ```yaml 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: [] ``` 2. Build and run the collector: `make otelcontribcol && ./bin/otelcontribcol_linux_amd64 --config ~/otelcol/monitoring_telemetry/config.yaml` 3. Produce some logs: ```console 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 ``` 4. Verify that metrics are produced: ```console 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:** <Describe the documentation added.> Added a respective section in Filelog receiver's docs. --------- Signed-off-by: ChrsMark <chrismarkou92@gmail.com>
Description:
This PR is the preliminary work for #31544.
The goal is to pass in the
component.TelemetrySettingsso as to use them later to report the filelog's state stats -> https://github.com/open-telemetry/opentelemetry-collector-contrib/pull/31544/files#diff-8e7ca9f2b21f494d9f8629f816ee7c6b170d4f6948e952d0aeb013c64cafed11R175-R206.ref: #31544 (review)
Link to tracking Issue: #31256
Testing:
Documentation: