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

[receiver/filelog][pkg/stanza] Prevent a lot of errors on windows if the path contains Junction #20732

Merged
merged 16 commits into from
Apr 26, 2023

Conversation

gillg
Copy link
Contributor

@gillg gillg commented Apr 6, 2023

Description:
By adding a dirty condition we prevent a known bug on windows to eval symlinks (see golang/go#39786 (comment))
On windows there is 2 concepts similars to Unix

  • Junction = mount point
  • Symlink = symbolic link

But the main difference is the junction appears as a file, like a symlink but should not be evaluated as a link in the golang method EvalSymlink (as we don't evaluate /mnt/mountpoint as /dev/sdb on unix)

So this dirty hack disable resolving any link on windows, but the loss is probably not very important. At least the file attributes will be avaluated on any circumctances and it's probably better to do it now as the stable release is coming.

Previous errors encountered without the fix

github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer.(*readerBuilder).build
        github.com/open-telemetry/opentelemetry-collector-contrib/pkg/[email protected]/fileconsumer/reader_factory.go:144
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer.(*readerFactory).newReader
        github.com/open-telemetry/opentelemetry-collector-contrib/pkg/[email protected]/fileconsumer/reader_factory.go:42
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer.(*Manager).newReader
        github.com/open-telemetry/opentelemetry-collector-contrib/pkg/[email protected]/fileconsumer/file.go:289
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer.(*Manager).makeReaders
        github.com/open-telemetry/opentelemetry-collector-contrib/pkg/[email protected]/fileconsumer/file.go:252
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer.(*Manager).consume
        github.com/open-telemetry/opentelemetry-collector-contrib/pkg/[email protected]/fileconsumer/file.go:139
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer.(*Manager).poll
        github.com/open-telemetry/opentelemetry-collector-contrib/pkg/[email protected]/fileconsumer/file.go:134
github.com/open-telemetry/opentelemetry-collector-contrib/pkg/stanza/fileconsumer.(*Manager).startPoller.func1
        github.com/open-telemetry/opentelemetry-collector-contrib/pkg/[email protected]/fileconsumer/file.go:103
2023-04-06T10:19:00.475Z        error   fileconsumer/reader_factory.go:144      resolve attributes: %!w(*errors.errorString=&{EvalSymlinks: too many links})    {"kind": "receiver", "name": "filelog/iis", "data_type": "logs", "component": "fileconsumer"}

Fixes: #21088

@gillg gillg requested a review from a team April 6, 2023 10:49
@gillg gillg requested a review from djaglowski as a code owner April 6, 2023 10:49
@runforesight
Copy link

runforesight bot commented Apr 6, 2023

Foresight Summary

    
Major Impacts

build-and-test duration(18 minutes 4 seconds) has decreased 28 minutes 21 seconds compared to main branch avg(46 minutes 25 seconds).
View More Details

⭕  build-and-test-windows workflow has finished in 5 seconds (31 minutes 52 seconds less than main branch avg.) and finished at 6th Apr, 2023.


Job Failed Steps Tests
windows-unittest-matrix -     🔗  N/A See Details
windows-unittest -     🔗  N/A See Details

✅  check-links workflow has finished in 45 seconds and finished at 6th Apr, 2023.


Job Failed Steps Tests
changed files -     🔗  N/A See Details
check-links -     🔗  N/A See Details

✅  telemetrygen workflow has finished in 1 minute and finished at 6th Apr, 2023.


Job Failed Steps Tests
build-dev -     🔗  N/A See Details
publish-latest -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details

❌  prometheus-compliance-tests workflow has finished in 1 minute 18 seconds (5 minutes 4 seconds less than main branch avg.) and finished at 6th Apr, 2023. 1 job failed.


Job Failed Steps Tests
prometheus-compliance-tests Run make otelcontribcol     🔗  N/A See Details

❌  changelog workflow has finished in 1 minute 30 seconds (44 seconds less than main branch avg.) and finished at 6th Apr, 2023. 1 job failed.


Job Failed Steps Tests
changelog Validate ./.chloggen/*.yaml changes     🔗  N/A See Details

❌  load-tests workflow has finished in 1 minute 33 seconds (8 minutes 49 seconds less than main branch avg.) and finished at 6th Apr, 2023. 1 job failed.


Job Failed Steps Tests
setup-environment Run make oteltestbedcol     🔗  N/A See Details
loadtest -     🔗  N/A See Details

❌  e2e-tests workflow has finished in 6 minutes 59 seconds (7 minutes 7 seconds less than main branch avg.) and finished at 6th Apr, 2023. 1 job failed.


Job Failed Steps Tests
kubernetes-test (v1.26.0) -     🔗  N/A See Details
kubernetes-test (v1.25.3) Build Docker Image     🔗  N/A See Details
kubernetes-test (v1.24.7) -     🔗  N/A See Details
kubernetes-test (v1.23.13) -     🔗  N/A See Details

❌  build-and-test workflow has finished in 18 minutes 4 seconds (28 minutes 21 seconds less than main branch avg.) and finished at 6th Apr, 2023. 7 jobs failed.


Job Failed Steps Tests
setup-environment -     🔗  N/A See Details
check-codeowners -     🔗  N/A See Details
checks -     🔗  N/A See Details
lint-matrix (receiver-0) -     🔗  N/A See Details
lint-matrix (receiver-1) Lint     🔗  N/A See Details
lint-matrix (processor) -     🔗  N/A See Details
lint-matrix (exporter) -     🔗  N/A See Details
lint-matrix (extension) -     🔗  N/A See Details
lint-matrix (connector) -     🔗  N/A See Details
lint-matrix (internal) -     🔗  N/A See Details
lint-matrix (other) -     🔗  N/A See Details
check-collector-module-version -     🔗  N/A See Details
build-examples Build Examples     🔗  N/A See Details
correctness-metrics -     🔗  N/A See Details
correctness-traces -     🔗  N/A See Details
integration-tests -     🔗  N/A See Details
unittest-matrix (1.20, receiver-0) -     🔗  N/A See Details
unittest-matrix (1.20, receiver-1) -     🔗  N/A See Details
unittest-matrix (1.20, processor) -     🔗  N/A See Details
unittest-matrix (1.20, exporter) -     🔗  N/A See Details
unittest-matrix (1.20, extension) -     🔗  N/A See Details
unittest-matrix (1.20, connector) -     🔗  N/A See Details
unittest-matrix (1.20, internal) -     🔗  N/A See Details
unittest-matrix (1.20, other) -     🔗  N/A See Details
unittest-matrix (1.19, receiver-0) -     🔗  N/A See Details
unittest-matrix (1.19, receiver-1) Run Unit Tests     🔗  N/A See Details
unittest-matrix (1.19, processor) -     🔗  N/A See Details
unittest-matrix (1.19, exporter) -     🔗  N/A See Details
unittest-matrix (1.19, extension) -     🔗  N/A See Details
unittest-matrix (1.19, connector) -     🔗  N/A See Details
unittest-matrix (1.19, internal) -     🔗  N/A See Details
unittest-matrix (1.19, other) -     🔗  N/A See Details
unittest (1.20) Interpret result     🔗  N/A See Details
unittest (1.19) Interpret result,Interpret result     🔗  N/A See Details
lint Interpret result     🔗  N/A See Details
cross-compile -     🔗  N/A See Details
windows-msi -     🔗  N/A See Details
build-package -     🔗  N/A See Details
publish-dev -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details
publish-check -     🔗  N/A See Details
rotate-milestone -     🔗  N/A See Details

🔎 See details on Foresight

*You can configure Foresight comments in your organization settings page.

@gillg gillg changed the title Prevent a lot of errors on windows if the path contains Junction [filelogreceiver] Prevent a lot of errors on windows if the path contains Junction Apr 6, 2023
@gillg gillg changed the title [filelogreceiver] Prevent a lot of errors on windows if the path contains Junction [receiver/filelog] Prevent a lot of errors on windows if the path contains Junction Apr 6, 2023
@gillg gillg changed the title [receiver/filelog] Prevent a lot of errors on windows if the path contains Junction [receiver/filelog][pkg/stanza] Prevent a lot of errors on windows if the path contains Junction Apr 6, 2023
@gillg
Copy link
Contributor Author

gillg commented Apr 24, 2023

@codeboten ready to be reviewed :)
The failed tests seems to be about a debian repo update issue.

@djaglowski djaglowski merged commit 84d9f48 into open-telemetry:main Apr 26, 2023
@github-actions github-actions bot added this to the next release milestone Apr 26, 2023
djaglowski added a commit that referenced this pull request Apr 26, 2023
…dows if the path contains Junction (#20732)"

This reverts commit 84d9f48.
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.

[receiver/filelog][pkg/stanza] Prevent a lot of errors on windows if the path contains Junction
3 participants