Skip to content

Document service log correlation #10454

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
Merged
merged 6 commits into from
Mar 24, 2023
Merged

Document service log correlation #10454

merged 6 commits into from
Mar 24, 2023

Conversation

SylvainJuge
Copy link
Member

@SylvainJuge SylvainJuge commented Mar 8, 2023 •
edited
Loading

Motivation/summary

Document service-level correlation for logs

part of elastic/apm#765

@mergify
Copy link
Contributor

mergify bot commented Mar 8, 2023

This pull request does not have a backport label. Could you fix it @SylvainJuge? 🙏
To fixup this pull request, you need to add the backport labels for the needed
branches, such as:

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

NOTE: backport-skip has been added to this pull request.

@mergify mergify bot added the backport-skip Skip notification from the automated backport with mergify label Mar 8, 2023
@SylvainJuge SylvainJuge mentioned this pull request Mar 8, 2023
Closed
5 tasks
@apmmachine
Copy link
Contributor

apmmachine commented Mar 8, 2023 •
edited
Loading

💚 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-03-24T00:30:52.507+0000

  • Duration: 9 min 52 sec

🤖 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 and publish the docker images.

  • /test windows : Build & tests on Windows.

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

@SylvainJuge SylvainJuge requested a review from a team March 20, 2023 12:17
dedemorton
dedemorton reviewed Mar 20, 2023
View reviewed changes
Copy link
Contributor

@dedemorton dedemorton left a comment

Choose a reason for hiding this comment

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

A few editorial changes. Otherwise looks good.

@SylvainJuge SylvainJuge requested a review from dedemorton March 21, 2023 09:16
simitt
simitt reviewed Mar 21, 2023
View reviewed changes
bmorelli25
bmorelli25 approved these changes Mar 22, 2023
View reviewed changes
Copy link
Member

@bmorelli25 bmorelli25 left a comment

Choose a reason for hiding this comment

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

Thank you! Should this be backported to 8.7?

@SylvainJuge SylvainJuge added backport-8.7 Automated backport with mergify and removed backport-skip Skip notification from the automated backport with mergify labels Mar 23, 2023
@SylvainJuge
Copy link
Member Author

@bmorelli25 Thanks for the review, I've added the 8.7 backport label, however it seems to be stuck on lint step, do you have any idea what could cause this ?

@kuisathaverat
Copy link
Contributor

It is not really stuck, the lint is not run, and the check is required, so the PR will never allow merging.

Lint is in https://github.com/elastic/apm-server/blob/main/.github/workflows/ci.yml#L26, but that workflow does not run on docs, and https://github.com/elastic/apm-server/blob/main/.github/workflows/ci-docs.yml does not have lint check. The solution is to split the lint to a separate workflow.

@bmorelli25
Copy link
Member

Thanks, @kuisathaverat! @SylvainJuge, While we wait for ci to be fixed, I've pushed an empty yaml file to this PR so that the full ci suite is triggered. This will allow us to merge this PR in the meantime.

@apmmachine

This comment was marked as resolved.

Sign in to view
@bmorelli25
Copy link
Member

@SylvainJuge it looks like some of your commits (and mine) are unsigned. I'm not sure how to remedy that. I finally figured out how to sign my commits but I'm not sure what to do since we each have unsigned commits.

@bmorelli25 bmorelli25 force-pushed the doc-log-service-correlation branch from 6934f43 to baf1c0a Compare March 24, 2023 00:23
@bmorelli25
Copy link
Member

Apologies for force pushing to your branch, but that seems to have signed all of the commits.

@bmorelli25 bmorelli25 enabled auto-merge (squash) March 24, 2023 00:30
@bmorelli25 bmorelli25 merged commit 71b5f2c into elastic:main Mar 24, 2023
mergify bot pushed a commit that referenced this pull request Mar 24, 2023
@SylvainJuge @dedemorton
@bmorelli25 @mergify
Document service log correlation (#10454)
49f3f6d 
* update docs

* Apply suggestions from code review

Co-authored-by: DeDe Morton <dede.morton@elastic.co>

* apply suggestions from code review

* remove duplication

* test

---------

Co-authored-by: DeDe Morton <dede.morton@elastic.co>
Co-authored-by: bmorelli25 <brandon.morelli@elastic.co>
(cherry picked from commit 71b5f2c)
@mergify mergify bot mentioned this pull request Mar 24, 2023
Closed
@SylvainJuge
Copy link
Member Author

Thanks for dealing with this @bmorelli25 , I don't mind force push if it allows to make changes move forward faster :-).

@SylvainJuge SylvainJuge deleted the doc-log-service-correlation branch March 24, 2023 13:22
bmorelli25 added a commit to bmorelli25/apm-server that referenced this pull request Mar 27, 2023
@SylvainJuge @dedemorton @bmorelli25
Document service log correlation (elastic#10454)
5461f08 
* update docs

* Apply suggestions from code review

Co-authored-by: DeDe Morton <dede.morton@elastic.co>

* apply suggestions from code review

* remove duplication

* test

---------

Co-authored-by: DeDe Morton <dede.morton@elastic.co>
Co-authored-by: bmorelli25 <brandon.morelli@elastic.co>
(cherry picked from commit 71b5f2c)
@bmorelli25 bmorelli25 mentioned this pull request Mar 27, 2023
Merged
@bmorelli25
Copy link
Member

💚 All backports created successfully

Status Branch Result
✅ 8.7

Questions ?

Please refer to the Backport tool documentation

bmorelli25 added a commit to bmorelli25/apm-server that referenced this pull request Mar 27, 2023
@SylvainJuge @dedemorton @bmorelli25
Document service log correlation (elastic#10454)
f149c59 
* update docs

* Apply suggestions from code review

Co-authored-by: DeDe Morton <dede.morton@elastic.co>

* apply suggestions from code review

* remove duplication

* test

---------

Co-authored-by: DeDe Morton <dede.morton@elastic.co>
Co-authored-by: bmorelli25 <brandon.morelli@elastic.co>
(cherry picked from commit 71b5f2c)
bmorelli25 added a commit that referenced this pull request Mar 27, 2023
@bmorelli25 @SylvainJuge
Document service log correlation (#10454) (#10539)
80446fb 
* update docs

* Apply suggestions from code review

Co-authored-by: DeDe Morton <dede.morton@elastic.co>

* apply suggestions from code review

* remove duplication

* test

---------

Co-authored-by: DeDe Morton <dede.morton@elastic.co>
Co-authored-by: bmorelli25 <brandon.morelli@elastic.co>
(cherry picked from commit 71b5f2c)

Co-authored-by: SylvainJuge <sylvain.juge@elastic.co>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@simitt simitt simitt left review comments

@bmorelli25 bmorelli25 bmorelli25 approved these changes

@dedemorton dedemorton Awaiting requested review from dedemorton

Assignees
No one assigned
Labels
backport-8.7 Automated backport with mergify docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
6 participants
@SylvainJuge @apmmachine @kuisathaverat @bmorelli25 @simitt @dedemorton