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

add span reparenting field child_ids #3679

Merged
merged 7 commits into from
Apr 22, 2020
Merged

Conversation

graphaelli
Copy link
Member

@graphaelli graphaelli commented Apr 22, 2020

Motivation/summary
Span reparenting intake. Closes #3422

Checklist

  • I have signed the Contributor License Agreement.
  • My code follows the style guidelines of this project (run make check-full for static code checks and linting)
  • I have rebased my changes on top of the latest master branch
  • I have made corresponding changes to the documentation
  • I have added tests that prove my fix is effective or that my feature works
  • New and existing unit tests pass locally with my changes

How to test these changes

Related issues

@graphaelli
Copy link
Member Author

graphaelli commented Apr 22, 2020

Even though it was in the proposal, I'm not sure about child_ids in the es document - child.id, which as keyword field would support >1 value, would be more symmetrical with the rest of the fields.

Copy link
Member

@axw axw left a comment

Choose a reason for hiding this comment

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

+1 on child.id, but would like to hear @simitt's thoughts given her recent ECS research. Otherwise LGTM.

@graphaelli
Copy link
Member Author

Good call. I pushed the change to child.id along with a changelog entry and will await feedback from @simitt.

Copy link
Contributor

@simitt simitt left a comment

Choose a reason for hiding this comment

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

IMO child is the right term here. What makes it confusing is that parent now doesn't actually mean parent anymore but can refer to any predecessor. A child's event can have another parent, that breaks my parent/child logic. Since parent.id is used for the waterfall UI (afaik) I don't think we could easily change that though, so LGTM.

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.

Update the parent ID of spans
3 participants