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.

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

[chore][receiver/journald] remove duplicate function NewFactory and pass checkapi #29005

Merged
merged 1 commit into from
Nov 14, 2023

Conversation

sakulali
Copy link
Contributor

@sakulali sakulali commented Nov 7, 2023

Description:
Remove duplicate function NewFactory and pass checkapi since we should only keep one NewFactory.

Link to tracking Issue:
#26304

Testing:
go run cmd/checkapi/main.go .
go test for journaldreceiver

Documentation:

@sakulali sakulali requested a review from djaglowski as a code owner November 7, 2023 11:21
@sakulali sakulali requested a review from a team November 7, 2023 11:21
@sakulali sakulali force-pushed the fix-journaldreceiver-api branch from b824060 to 6fb32dd Compare November 7, 2023 11:36
Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what is being solved here.

Journald is a linux specific component, so this change introduces a factory for non-linux usage, but it does nothing? It seems like it. may cause other problems.

Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nevermind my previous question. I understand the changes now.

@sakulali sakulali force-pushed the fix-journaldreceiver-api branch from b4e5b94 to 60623b4 Compare November 10, 2023 15:52
@sakulali
Copy link
Contributor Author

Nevermind my previous question. I understand the changes now.

Thanks for reviews @djaglowski! I should make it clear in the description that we should only keep one NewFactory.

@sakulali
Copy link
Contributor Author

CI seems go wrong(seems not related with this PR), but i have no idea about that.

stanza/operator/input/udp/udp.go:142: File is not `gofmt`-ed with `-s` `-r 'interface{} -> any'` (gofmt)
level=info msg="[runner] processing took 32.17115ms with stages: exclude-rules: 11.786536ms, identifier_marker: 8.651115ms, path_prettifier: 4.33792ms, autogenerated_exclude: 3.482249ms, nolint: 3.399537ms, skip_dirs: 390.69µs, source_code: 48.711µs, cgo: 37.53µs, filename_unadjuster: 19.466µs, max_same_issues: 5.27µs, uniq_by_line: 3.145µs, max_from_linter: 2.225µs, max_per_file_from_linter: 1.694µs, path_prefixer: 1.634µs, path_shortener: 851ns, severity-rules: 561ns, sort_results: 501ns, fixer: 480ns, skip_files: 392ns, exclude: 342ns, diff: 301ns"
level=info msg="[runner] linters took 15.6624762s with stages: goanalysis_metalinter: 15.630148672s"
level=info msg="File cache stats: 229 entries of total size 1.1MiB"
			New: func() interface{} {
level=info msg="Memory: 211 samples, avg is 403.6MB, max is 699.6MB"
level=info msg="Execution took 22.229189021s"
make[2]: *** [../../Makefile.Common:177: lint] Error 1
make[2]: Leaving directory '/home/runner/work/opentelemetry-collector-contrib/opentelemetry-collector-contrib/pkg/stanza'

@sakulali sakulali force-pushed the fix-journaldreceiver-api branch from 60623b4 to d2951a5 Compare November 11, 2023 05:45
@djaglowski djaglowski merged commit d2b5291 into open-telemetry:main Nov 14, 2023
82 checks passed
@github-actions github-actions bot added this to the next release milestone Nov 14, 2023
@sakulali sakulali deleted the fix-journaldreceiver-api branch November 18, 2023 06:47
RoryCrispin pushed a commit to ClickHouse/opentelemetry-collector-contrib that referenced this pull request Nov 24, 2023
…ass checkapi (open-telemetry#29005)

**Description:**
Remove duplicate function NewFactory and pass checkapi since we should
only keep one NewFactory.

**Link to tracking Issue:**

open-telemetry#26304

**Testing:**
go run cmd/checkapi/main.go .
go test for journaldreceiver

**Documentation:**

Signed-off-by: sakulali <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants