Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

[pkg/stanza] Switch from SugaredLogger to Logger #32177

Merged
merged 1 commit into from
May 16, 2024
Merged

[pkg/stanza] Switch from SugaredLogger to Logger #32177

merged 1 commit into from
May 16, 2024

Conversation

djaglowski
Copy link
Member

@djaglowski djaglowski commented Apr 4, 2024 •
edited
Loading

#32662 updated many exported functions to accept component.TelemetrySettings instead of zap.SugaredLogger. This PR continues from there by passing component.TelemetrySettings deeper into the inner packages, and migrates from using zap.SugaredLogger to instead using zap.Logger (as provided by component.TelemetrySettings).

@djaglowski djaglowski force-pushed the pkg-stanza-rm-sugared branch 3 times, most recently from 7bb8633 to 1a5a872 Compare April 5, 2024 16:13
@github-actions github-actions bot requested a review from atoulme April 5, 2024 16:14
@github-actions github-actions bot added the processor/logstransform Logs Transform processor label Apr 5, 2024
@github-actions github-actions bot requested a review from dehaansa April 5, 2024 16:24
@github-actions GitHub Actions
Copy link
Contributor

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

@github-actions github-actions bot added the Stale label Apr 20, 2024
@djaglowski djaglowski removed the Stale label Apr 22, 2024
@ChrsMark ChrsMark mentioned this pull request Apr 24, 2024
Merged
djaglowski pushed a commit that referenced this pull request Apr 24, 2024
@ChrsMark
[breaking][pkg/stanza] Pass TelemetrySettings to Build of Builder int…
6cefabc 
…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 <[email protected]>
@djaglowski djaglowski force-pushed the pkg-stanza-rm-sugared branch 3 times, most recently from 4bbbdf8 to 8b59da2 Compare April 26, 2024 14:55
@djaglowski djaglowski changed the title [DO NOT MERGE][pkg/stanza] Switch from SugaredLogger to Logger [pkg/stanza] Switch from SugaredLogger to Logger Apr 26, 2024
@djaglowski djaglowski force-pushed the pkg-stanza-rm-sugared branch 2 times, most recently from 4af70a1 to 74c9667 Compare April 26, 2024 16:55
@djaglowski djaglowski marked this pull request as ready for review April 26, 2024 17:41
@djaglowski djaglowski requested a review from a team as a code owner April 26, 2024 17:41
@djaglowski
Copy link
Member Author

This is just waiting on a review @open-telemetry/collector-contrib-approvers @ChrsMark

mx-psi
mx-psi reviewed May 13, 2024
View reviewed changes
.chloggen/pkg-stanza-rm-sugared.yaml Show resolved Hide resolved
pkg/stanza/fileconsumer/internal/header/reader.go Outdated Show resolved Hide resolved
pkg/stanza/fileconsumer/internal/reader/factory.go Show resolved Hide resolved
pkg/stanza/operator/helper/operator.go Outdated Show resolved Hide resolved
pkg/stanza/pipeline/directed.go Outdated Show resolved Hide resolved
pkg/stanza/testutil/util.go Outdated Show resolved Hide resolved
ChrsMark
ChrsMark reviewed May 14, 2024
View reviewed changes
pkg/stanza/fileconsumer/file.go Outdated Show resolved Hide resolved
.chloggen/pkg-stanza-rm-sugared-api.yaml Outdated Show resolved Hide resolved
pkg/stanza/pipeline/directed.go Outdated Show resolved Hide resolved
@djaglowski djaglowski merged commit d78d7bb into open-telemetry:main May 16, 2024
161 of 162 checks passed
@djaglowski djaglowski deleted the pkg-stanza-rm-sugared branch May 16, 2024 19:49
@github-actions github-actions bot added this to the next release milestone May 16, 2024
dmitryax added a commit to signalfx/splunk-otel-collector that referenced this pull request May 22, 2024
@dmitryax
[chore] [receiver/scriptedinput] Use correct logger
204e02a 
Remove usage of incorrect logger inherited from stanza InputOperator and removed in 0.101.0 as part of open-telemetry/opentelemetry-collector-contrib#32177
@dmitryax dmitryax mentioned this pull request May 22, 2024
Merged
dmitryax added a commit to signalfx/splunk-otel-collector that referenced this pull request May 22, 2024
@dmitryax
[chore] [receiver/scriptedinput] Use correct logger (#4867)
995eae8 
Remove usage of incorrect logger inherited from stanza InputOperator and removed in 0.101.0 as part of open-telemetry/opentelemetry-collector-contrib#32177
@fordneild
Copy link

This seems to have broken drop_quiet and send_quiet from what I can tell. Now my failed "move" operator is logging even though i have it on send_quiet

@mx-psi
Copy link
Member

mx-psi commented Aug 19, 2024

This seems to have broken drop_quiet and send_quiet from what I can tell. Now my failed "move" operator is logging even though i have it on send_quiet

@fordneild would you mind opening a separate issue for this?

cc @djaglowski

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@mx-psi mx-psi mx-psi approved these changes

@ChrsMark ChrsMark ChrsMark left review comments

@atoulme atoulme Awaiting requested review from atoulme

@dehaansa dehaansa Awaiting requested review from dehaansa

@sinkingpoint sinkingpoint Awaiting requested review from sinkingpoint

Assignees

@TylerHelmuth TylerHelmuth

Labels
pkg/stanza processor/logstransform Logs Transform processor receiver/filelog receiver/namedpipe receiver/otlpjsonfile receiver/zookeeper
Projects
None yet
Milestone
v0.101.0
Development

Successfully merging this pull request may close these issues.
5 participants
@djaglowski @fordneild @mx-psi @ChrsMark @TylerHelmuth