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

Revert "[filebeat] Fix long filepaths in diagnostics exceeding max path limits on Windows" #40980

Merged
merged 1 commit into from
Sep 25, 2024
Merged

Revert "[filebeat] Fix long filepaths in diagnostics exceeding max path limits on Windows" #40980

merged 1 commit into from
Sep 25, 2024

Conversation

jlind23
Copy link
Collaborator

@jlind23 jlind23 commented Sep 25, 2024

Reverts #40909

@aleksmaus I am sorry I have to revert this but it introduced a problem with Elastic Agent where lock file on Windows remain and diagnostic are not being generated at all.

cc @ycombinator @cmacknz

@jlind23 jlind23 requested review from a team as code owners September 25, 2024 05:37
@botelastic botelastic bot added the needs_team Indicates that the issue/PR needs a Team:* label label Sep 25, 2024
@jlind23 jlind23 enabled auto-merge (squash) September 25, 2024 05:37
@mergify Mergify
Copy link
Contributor

mergify bot commented Sep 25, 2024

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @jlind23? 🙏.
For such, you'll need to label your PR with:

  • The upcoming major version of the Elastic Stack
  • The upcoming minor version of the Elastic Stack (if you're not pushing a breaking change)

To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

  • backport-8./d is the label to automatically backport to the 8./d branch. /d is the digit

@mergify Mergify
Copy link
Contributor

mergify bot commented Sep 25, 2024

backport-8.x has been added to help with the transition to the new branch 8.x.
If you don't need it please use backport-skip label and remove the backport-8.x label.

@mergify mergify bot added the backport-8.x Automated backport to the 8.x branch with mergify label Sep 25, 2024
@jlind23 jlind23 added the Team:Elastic-Agent Label for the Agent team label Sep 25, 2024
@botelastic botelastic bot removed the needs_team Indicates that the issue/PR needs a Team:* label label Sep 25, 2024
@elasticmachine
Copy link
Collaborator

Pinging @elastic/elastic-agent (Team:Elastic-Agent)

@jlind23 jlind23 added backport-skip Skip notification from the automated backport with mergify and removed backport-8.x Automated backport to the 8.x branch with mergify labels Sep 25, 2024
@efd6
Copy link
Contributor

efd6 commented Sep 25, 2024

@jlind23 What was the mechanism for that?

@jlind23
Copy link
Collaborator Author

jlind23 commented Sep 25, 2024 •
edited
Loading

@efd6
On windows tests are always failing as you can see on the Elastic Agent main branch:
https://buildkite.com/elastic/elastic-agent-extended-testing/builds/3027#_
If you take one of the recent build failure on elastic agent main you'll see the problem.
Looks like the process PID on windows are always changing and we end up with a problem like:
image

Then our integration framework will try to generate a diagnostic but this will eventually fail with the error below:

C:\Users\windows\agent\build\diagnostics\TestLongRunningAgentForLeaks-2024-09-23T08-44-44Z-install-directory.zip: walking C:\Program Files\Elastic\Agent to create zip: read C:\Program Files\Elastic\Agent\data\agent.lock: The process cannot access the file because another process has locked a portion of the file.

@efd6
Copy link
Contributor

efd6 commented Sep 25, 2024

OK, but I don't see how this change would cause that.

@jlind23
Copy link
Collaborator Author

jlind23 commented Sep 25, 2024

@ycombinator can tell you more as he worked on the analysis yesterday and found out that this specific commit was the one causing problem.

@ycombinator
Copy link
Contributor

@ycombinator can tell you more as he worked on the analysis yesterday and found out that this specific commit was the one causing problem.

We are suspecting that this commit might be the problem but haven't proved it definitively yet. @efd6 I'll ping you on the Slack thread where we've been discussing this issue for the past couple of days so I don't have to duplicate the conversation and findings in both places.

@aleksmaus aleksmaus mentioned this pull request Sep 25, 2024
Closed
2 tasks
@jlind23 jlind23 mentioned this pull request Sep 25, 2024
Merged
@ycombinator
Copy link
Contributor

Let's merge this to see if it unblocks Agent CI. If it doesn't we'll revert this PR (revert the revert).

@ycombinator ycombinator merged commit dbbfb5b into main Sep 25, 2024
27 of 30 checks passed
@ycombinator ycombinator deleted the revert-40909-fix/filebeat_diagnostics_long_filename branch September 25, 2024 17:34
@ycombinator ycombinator mentioned this pull request Sep 25, 2024
Merged
VihasMakwana added a commit that referenced this pull request Sep 26, 2024
@VihasMakwana
Squashed commit of the following:
e72b7e6 
commit 16ecdee682027804deddd108a0229fbfe57a9013
Author: Vihas Makwana <vihas.makwana@elastic.co>
Date:   Thu Sep 26 11:49:02 2024 +0530

    go.mod

commit c2aa64b
Merge: b92b041 cc9b664
Author: Vihas Makwana <vihas.makwana@elastic.co>
Date:   Thu Sep 26 11:29:28 2024 +0530

    Merge branch 'beats-otel-collector' into esotel-beats

commit b92b041
Merge: 126aa22 7cc59c9
Author: Vihas Makwana <vihas.makwana@elastic.co>
Date:   Thu Sep 26 11:22:14 2024 +0530

    Merge branch 'lee/fbreceiver' into esotel-beats

commit 7cc59c9
Merge: 6aeba30 a08e9c1
Author: Vihas Makwana <vihas.makwana@elastic.co>
Date:   Thu Sep 26 11:21:51 2024 +0530

    Merge branch 'fbreceiver' of github.com:leehinman/beats into lee/fbreceiver

commit a08e9c1
Author: Lee Hinman <lee.e.hinman@elastic.co>
Date:   Wed Sep 25 17:21:28 2024 -0500

    add debug and enable benchmark input on windows

commit bb7aaf2
Author: Lee E. Hinman <lee.e.hinman@elastic.co>
Date:   Wed Sep 25 14:16:57 2024 -0500

    increase timeout for fbreceiver test, and make sure logs are current

commit f493764
Author: Lee E. Hinman <lee.e.hinman@elastic.co>
Date:   Wed Sep 25 11:22:46 2024 -0500

    add slices and test cases for mapstrToPcommonMap

commit 71f6deb
Author: Blake Rouse <blake.rouse@elastic.co>
Date:   Tue Sep 24 20:27:55 2024 -0400

    Update config_test.go

commit f354cb4
Author: Lee E Hinman <57081003+leehinman@users.noreply.github.com>
Date:   Tue Sep 24 15:09:50 2024 -0500

    Update x-pack/filebeat/fbreceiver/config.go

    Co-authored-by: Blake Rouse <blake.rouse@elastic.co>

commit d41c801
Author: Lee E Hinman <57081003+leehinman@users.noreply.github.com>
Date:   Tue Sep 24 15:09:40 2024 -0500

    Update x-pack/filebeat/fbreceiver/config.go

    Co-authored-by: Blake Rouse <blake.rouse@elastic.co>

commit 9103b0b
Author: Lee E. Hinman <lee.e.hinman@elastic.co>
Date:   Tue Sep 24 14:33:49 2024 -0500

    update notice.txt

commit bd0cbfd
Author: Lee E. Hinman <lee.e.hinman@elastic.co>
Date:   Tue Sep 24 14:22:21 2024 -0500

    linter fixes

commit 38715e2
Author: Lee E. Hinman <lee.e.hinman@elastic.co>
Date:   Thu Sep 19 14:20:57 2024 -0500

    Add filebeat receiver and otel consumer output

commit dbbfb5b
Author: Julien Lind <julien.lind@elastic.co>
Date:   Wed Sep 25 19:34:34 2024 +0200

    Revert "[filebeat] Fix long filepaths in diagnostics exceeding max path limit…" (#40980)

    This reverts commit d82fe9a.

commit 126aa22
Author: Vihas Makwana <vihas.makwana@elastic.co>
Date:   Wed Sep 25 19:12:09 2024 +0530

    chore: initial commit

commit 378b796
Author: Olga Naydyonock <olga.naidjonoka@elastic.co>
Date:   Wed Sep 25 13:31:35 2024 +0300

    added pre-commit to x-pack/agentbeat (#40965)

commit 1e6364a
Author: Klim Markelov <klim.markelov@elastic.co>
Date:   Wed Sep 25 12:26:16 2024 +0200

    Change codeowners from ent-search-application to app-search-team (#40963)

commit 6aeba30
Author: Blake Rouse <blake.rouse@elastic.co>
Date:   Tue Sep 24 20:27:55 2024 -0400

    Update config_test.go

commit 55e5375
Author: Lee E Hinman <57081003+leehinman@users.noreply.github.com>
Date:   Tue Sep 24 15:09:50 2024 -0500

    Update x-pack/filebeat/fbreceiver/config.go

    Co-authored-by: Blake Rouse <blake.rouse@elastic.co>

commit d7a5211
Author: Lee E Hinman <57081003+leehinman@users.noreply.github.com>
Date:   Tue Sep 24 15:09:40 2024 -0500

    Update x-pack/filebeat/fbreceiver/config.go

    Co-authored-by: Blake Rouse <blake.rouse@elastic.co>

commit 38d1b5d
Author: Lee E. Hinman <lee.e.hinman@elastic.co>
Date:   Tue Sep 24 14:33:49 2024 -0500

    update notice.txt

commit 931d661
Author: Lee E. Hinman <lee.e.hinman@elastic.co>
Date:   Tue Sep 24 14:22:21 2024 -0500

    linter fixes

commit a74d26f
Author: Lee E. Hinman <lee.e.hinman@elastic.co>
Date:   Thu Sep 19 14:20:57 2024 -0500

    Add filebeat receiver and otel consumer output
@mergify mergify bot mentioned this pull request Sep 26, 2024
Merged
@efd6 efd6 mentioned this pull request Oct 9, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@pierrehilbert pierrehilbert pierrehilbert approved these changes

@ycombinator ycombinator ycombinator approved these changes

@khushijain21 khushijain21 Awaiting requested review from khushijain21 khushijain21 is a code owner automatically assigned from elastic/elastic-agent-data-plane

@VihasMakwana VihasMakwana Awaiting requested review from VihasMakwana VihasMakwana is a code owner automatically assigned from elastic/elastic-agent-data-plane

Assignees

@jlind23 jlind23

Labels
backport-skip Skip notification from the automated backport with mergify Team:Elastic-Agent Label for the Agent team
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
5 participants
@jlind23 @elasticmachine @efd6 @ycombinator @pierrehilbert