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 handler to produce a full trace with child spans #10

Merged
merged 5 commits into from
Sep 8, 2022

Conversation

tdarwin
Copy link
Contributor

@tdarwin tdarwin commented Aug 30, 2022

Which problem is this PR solving?

The current version of the handler does not produce what Honeycomb recognizes as a trace, so there is no waterfall view of processes, access to use the node object in queries, and very minimal bubble up capabilities.

Short description of the changes

This updates the product to be a full trace, where the compile, converge, and handler phases are all shown as child spans, and the converge phase is broken down into spans per cookbook, recipe, and resource.

All objects have access to a subset of the node object that make bubble-up and querying more interesting, and users are provided with a mechanism to ensure that node attributes/objects that they care about can be included in that data as well.

Notes

There's some funky code to make this work as a handler that's part of a cookbook, but it does. If I were committing this as something I wanted to be part of Chef itself, there would be cleaner ways to do certain things.

Resulting Images

Here are some screen captures of this handler in action in waterfall mode, query + heatmap, and bubble-up:
image (4)
image (3)
image (2)
image (1)

tdarwin and others added 4 commits August 30, 2022 11:29
… in the default recipe to provide a lot more detail in the

Signed-off-by: Davin Taddeo <davin@davintaddeo.com>
…hef Client version 17.

Signed-off-by: Davin Taddeo <davin@honeycomb.io>
Signed-off-by: Davin Taddeo <davin@honeycomb.io>
…e obviously examples.

Signed-off-by: Davin Taddeo <davin@honeycomb.io>
@tdarwin tdarwin requested review from a team and MikeGoldsmith August 30, 2022 22:05
@tdarwin
Copy link
Contributor Author

tdarwin commented Aug 30, 2022

As a note. I've only tested this with Chef/CINC client version 17.10 so far.

@pkanal pkanal added the status: oncall Flagged for awareness from Honeycomb Telemetry Oncall label Aug 31, 2022
@tdarwin tdarwin added status: review needed Changes need review. and removed status: oncall Flagged for awareness from Honeycomb Telemetry Oncall labels Aug 31, 2022
@vreynolds vreynolds added the status: oncall Flagged for awareness from Honeycomb Telemetry Oncall label Sep 2, 2022
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

Looks really good, thanks @tdarwin 👍🏻

I've left some feedback to remove an unused func, remove commented out code and styling things.

README.md Outdated Show resolved Hide resolved
libraries/honeycomb.rb Outdated Show resolved Hide resolved
libraries/honeycomb.rb Outdated Show resolved Hide resolved
libraries/honeycomb.rb Outdated Show resolved Hide resolved
libraries/honeycomb.rb Outdated Show resolved Hide resolved
recipes/default.rb Outdated Show resolved Hide resolved
recipes/default.rb Outdated Show resolved Hide resolved
recipes/default.rb Outdated Show resolved Hide resolved
libraries/honeycomb.rb Outdated Show resolved Hide resolved
@MikeGoldsmith MikeGoldsmith added type: enhancement status: revision needed Waiting for response to changes requested. version: bump minor A PR that adds behavior, but is backwards-compatible. and removed status: review needed Changes need review. labels Sep 6, 2022
@MikeGoldsmith MikeGoldsmith self-assigned this Sep 6, 2022
@tdarwin
Copy link
Contributor Author

tdarwin commented Sep 8, 2022

@MikeGoldsmith Thanks for all the suggestions and tweaks and nit-picks. I'll put out another commit soon to clean stuff up. might not be until Friday when I'm back from this conference.

Signed-off-by: Davin Taddeo <davin@honeycomb.io>
Copy link
Contributor

@MikeGoldsmith MikeGoldsmith left a comment

Choose a reason for hiding this comment

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

This looks good, thanks @tdarwin 👍🏻

@MikeGoldsmith MikeGoldsmith merged commit d81a4ca into honeycombio:main Sep 8, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
status: oncall Flagged for awareness from Honeycomb Telemetry Oncall status: revision needed Waiting for response to changes requested. type: enhancement version: bump minor A PR that adds behavior, but is backwards-compatible.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants