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

Adds USDT probes to Dropshot #160

Merged
merged 7 commits into from
Nov 5, 2021
Merged

Adds USDT probes to Dropshot #160

merged 7 commits into from
Nov 5, 2021

Conversation

bnaecker
Copy link
Contributor

  • Adds the RequestInfo and ResponseInfo structs to package
    information available to DTrace
  • Adds USDT probes request_start and request_finish
  • Exposes the probes in the http_request_handle_wrap function
  • Adds feature flag for optionally enabling probes
  • Adds notes to README about the probes, how to enable and see via
    DTrace

- Adds the `RequestInfo` and `ResponseInfo` structs to package
  information available to DTrace
- Adds USDT probes `request_start` and `request_finish`
- Exposes the probes in the `http_request_handle_wrap` function
- Adds feature flag for optionally enabling probes
- Adds notes to README about the probes, how to enable and see via
  DTrace
@bnaecker bnaecker requested a review from ahl October 19, 2021 05:14
Copy link
Collaborator

@davepacheco davepacheco left a comment

Choose a reason for hiding this comment

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

Very cool!

dropshot/src/server.rs Outdated Show resolved Hide resolved
dropshot/src/server.rs Outdated Show resolved Hide resolved
dropshot/src/server.rs Outdated Show resolved Hide resolved
dropshot/src/lib.rs Outdated Show resolved Hide resolved
dropshot/src/lib.rs Outdated Show resolved Hide resolved
@davepacheco
Copy link
Collaborator

Looking good! Would you also mind adding a note to the changelog?

@bnaecker
Copy link
Contributor Author

bnaecker commented Nov 5, 2021

@davepacheco Yes, I'll add notes to the changelog, and thanks for the review!

I'm actually planning to update this a bit, in a way we've discussed previously. I think it's a bit better to perform registration in the server construction, so that no one needs to remember to do this or change existing code to get probes, and store the result of that in a field of the server. Instead of a separate call, one can check the field to assert probes have been enabled (or behave however one chooses).

caller

- Moves the call to `usdt::register_probes()` back into the internals of
  building/starting a server, to ensure it's always done if possible.
  Information about the result of that call is stored in the
  `HttpServer`, so that caller's can decide how to handle failure.
- Remove call to `usdt::register_probes()` in accordance with above
- Adds notes about USDT probes to changelog.
@bnaecker bnaecker merged commit 3ce5aa2 into main Nov 5, 2021
@bnaecker bnaecker deleted the feature/usdt-probes branch November 5, 2021 22:25
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants