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

update elastic-agent-shipper-client deps #34596

Merged
merged 6 commits into from
Feb 27, 2023

Conversation

fearful-symmetry
Copy link
Contributor

What does this PR do?

This updates the elastic-agent-shipper-client deps to pull in the recent bugfixes.

Why is it important?

This upstream change fixes a number of bugs.

@fearful-symmetry fearful-symmetry added the Team:Elastic-Agent Label for the Agent team label Feb 15, 2023
@fearful-symmetry fearful-symmetry self-assigned this Feb 15, 2023
@fearful-symmetry fearful-symmetry requested a review from a team as a code owner February 15, 2023 22:01
@fearful-symmetry fearful-symmetry requested review from rdner and cmacknz and removed request for a team February 15, 2023 22:01
@elasticmachine
Copy link
Collaborator

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

@botelastic botelastic bot added needs_team Indicates that the issue/PR needs a Team:* label and removed needs_team Indicates that the issue/PR needs a Team:* label labels Feb 15, 2023
@mergify
Copy link
Contributor

mergify bot commented Feb 15, 2023

This pull request does not have a backport label.
If this is a bug or security fix, could you label this PR @fearful-symmetry? 🙏.
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-v8./d.0 is the label to automatically backport to the 8./d branch. /d is the digit

@cmacknz cmacknz added the backport-v8.7.0 Automated backport with mergify label Feb 15, 2023
@fearful-symmetry
Copy link
Contributor Author

Alright, CI errors appear to be actually related, looking over them...

@elasticmachine
Copy link
Collaborator

elasticmachine commented Feb 15, 2023

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2023-02-24T20:51:37.771+0000

  • Duration: 46 min 36 sec

❕ Flaky test report

No test was executed to be analysed.

🤖 GitHub comments

Expand to view the GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • /package : Generate the packages and run the E2E tests.

  • /beats-tester : Run the installation tests with beats-tester.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

@fearful-symmetry
Copy link
Contributor Author

Alright, I'm not 100% sure if some of these errors. The "returns error if failed to convert metadata" and "returns error if failed to convert fields" tests now return an event with the fields {"field":{}} and I'm not sure if we need/want that to be an error condition? It looks like a few other tests are also failing because we're expected to error out on empty objects. Should that be an error condition? @cmacknz

@cmacknz
Copy link
Member

cmacknz commented Feb 16, 2023

{"field":{}} is valid JSON so it should not be an error. We should correctly serialize and pass this through. It is entirely possible we are trying to ship a response from an external API that includes an empty JSON object in its response.

The protojson package serializes the message to the structure below which is syntactically valid JSON for example:

            {
              "timestamp": "2023-02-16T18:54:25Z",
              "source": {},
              "dataStream": {},
              "fields": {
                "data": {
                  "field": {
                    "structValue": {}
                  }
                }
              }
            }

@fearful-symmetry
Copy link
Contributor Author

Yah, that's what I thought, just wanted to make sure.

@fearful-symmetry
Copy link
Contributor Author

Want to update the PR with fixes before the long weekend, but right now we're in a weird state where we're not testing certain things like retries/dropped events, since the marshaling code is much more aggressive with not erring out. Will have to re-do some of the mock shipper server so I can get events to drop.

I also suspect that since the event marshaler is more capable of handling map types, there might be some redundant code. Not sure if we want to deal with that in this PR or do a follow-up.

@rdner rdner removed their request for review February 22, 2023 08:27
@mergify
Copy link
Contributor

mergify bot commented Feb 23, 2023

This pull request is now in conflicts. Could you fix it? 🙏
To fixup this pull request, you can check out it locally. See documentation: https://help.github.com/articles/checking-out-pull-requests-locally/

git fetch upstream
git checkout -b shipper-client-update upstream/shipper-client-update
git merge upstream/main
git push upstream shipper-client-update

@fearful-symmetry
Copy link
Contributor Author

/test

1 similar comment
@fearful-symmetry
Copy link
Contributor Author

/test

@fearful-symmetry
Copy link
Contributor Author

E2E failures are unrelated, merging

@fearful-symmetry fearful-symmetry merged commit b5c00fd into elastic:main Feb 27, 2023
mergify bot pushed a commit that referenced this pull request Feb 27, 2023
* update elastic-agent-shipper-client deps

* remove tests depending on failure

* remove debug lines

* update tests

* try to make linter happy

(cherry picked from commit b5c00fd)
ycombinator pushed a commit that referenced this pull request Apr 5, 2023
* update elastic-agent-shipper-client deps

* remove tests depending on failure

* remove debug lines

* update tests

* try to make linter happy

(cherry picked from commit b5c00fd)
ycombinator pushed a commit that referenced this pull request Apr 6, 2023
* update elastic-agent-shipper-client deps

* remove tests depending on failure

* remove debug lines

* update tests

* try to make linter happy

(cherry picked from commit b5c00fd)

Co-authored-by: Alex K <[email protected]>
chrisberkhout pushed a commit that referenced this pull request Jun 1, 2023
* update elastic-agent-shipper-client deps

* remove tests depending on failure

* remove debug lines

* update tests

* try to make linter happy
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-v8.7.0 Automated backport with mergify Team:Elastic-Agent Label for the Agent team
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants