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

Writing an instrumentation log into the package directory causes spam #48885

Closed
natebosch opened this issue Apr 26, 2022 · 5 comments
Closed
Assignees
Labels
analyzer-lsp Issues with analysis server's support of Language Server Protocol area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request

Comments

@natebosch
Copy link
Member

1650992695603:Watch:<unknown>:/usr/local/google/home/nbosch/repos/dart_repro/instrumentation_log:modify
1650992695603:Watch:<unknown>:/usr/local/google/home/nbosch/repos/dart_repro/instrumentation_log:modify
1650992695604:Watch:<unknown>:/usr/local/google/home/nbosch/repos/dart_repro/instrumentation_log:modify
1650992695604:Watch:<unknown>:/usr/local/google/home/nbosch/repos/dart_repro/instrumentation_log:modify
1650992695604:Watch:<unknown>:/usr/local/google/home/nbosch/repos/dart_repro/instrumentation_log:modify
1650992695604:Watch:<unknown>:/usr/local/google/home/nbosch/repos/dart_repro/instrumentation_log:modify
1650992695605:Watch:<unknown>:/usr/local/google/home/nbosch/repos/dart_repro/instrumentation_log:modify

There is some useful logging at the beginning, but then the file just becomes neverending modifications because it logs each change it makes.

We should either warn when you use an instrumentation log argument where this will happen, or filter out these events from the log.

@natebosch natebosch added area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. analyzer-lsp Issues with analysis server's support of Language Server Protocol labels Apr 26, 2022
@bwilkerson
Copy link
Member

@DanTup

@bwilkerson bwilkerson added the P3 A lower priority bug or feature request label Apr 26, 2022
@DanTup
Copy link
Collaborator

DanTup commented Apr 27, 2022

@bwilkerson I could detect/warn on this in the VS Code extension (or LSP server), but I suspect the same happens for the original protocol too (where I'm not sure we can easily get a warning to the user).

Filtering out watch events for the log itself seems seems sensible, although I think it means InstrumentationLogAdapter (created here) may need to also be given the path directly to exclude. WDYT?

@bwilkerson
Copy link
Member

If we filter out any watch events for the log itself, then I'm not sure we need to warn the user. If we do need to notify them, then we can use the server.error notification to do so when using the legacy protocol.

Let's start by filtering. And I wasn't assigning this to you, so don't feel obligated to work on it.

@bwilkerson bwilkerson added P2 A bug or feature request we're likely to work on and removed P3 A lower priority bug or feature request labels Apr 27, 2022
@natebosch
Copy link
Member Author

This does have an easy workaround so I would consider it low priority.

@bwilkerson bwilkerson added P3 A lower priority bug or feature request and removed P2 A bug or feature request we're likely to work on labels Apr 28, 2022
@DanTup
Copy link
Collaborator

DanTup commented Dec 12, 2022

I just (re)discovered this issue today... I thought we had fixed it previously, because I'm frequently writing instrumentation logs into my project folders and haven't seen it until today. However today I'm using a Docker container to debug a (suspected) Linux issue and saw this. I wonder if it's only happening on Linux (edit: that seems to be the case.. perhaps the way the log is flushed doesn't trigger these events on macOS?).

Anyway, I'll try to fix this, because logging outside of the workspace folder seems to fail when using a devcontainer and that's preventing debugging this other issue.

@DanTup DanTup self-assigned this Dec 12, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
analyzer-lsp Issues with analysis server's support of Language Server Protocol area-analyzer Use area-analyzer for Dart analyzer issues, including the analysis server and code completion. P3 A lower priority bug or feature request
Projects
None yet
Development

No branches or pull requests

3 participants