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.

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

view: 👀 introduce more tracing instrumentation #4349

Merged
merged 2 commits into from
May 8, 2024
Merged

view: 👀 introduce more tracing instrumentation #4349

merged 2 commits into from
May 8, 2024

Conversation

cratelyn
Copy link
Contributor

@cratelyn cratelyn commented May 7, 2024

describe your changes

this introduces some additional tracing instrumentation, used while debugging
integration with the view server in mock consensus tests. because these events
may be useful more broadly, and aren't strictly tied to testing, this is
cherry-picked out of that work for separate consideration.

issue ticket number and link

checklist before requesting a review

  • if this code contains consensus-breaking changes, i have added the "consensus-breaking" label. otherwise, i declare my belief that there are not consensus-breaking changes, for the following reason:

this branch only makes changes to tracing spans and events. as this is
only altering telemetry, this is not consensus-breaking.

@cratelyn
view: 👀 introduce more tracing instrumentation
b4b747b 
NB: this adds a `tap` dependency to the `penumbra-view` crate too.
@cratelyn cratelyn added A-node Area: System design and implementation for node software C-enhancement Category: an enhancement to the codebase A-telemetry Area: Metrics, logging, and other observability-related features labels May 7, 2024
@cratelyn cratelyn added this to the Sprint 6 milestone May 7, 2024
@cratelyn cratelyn self-assigned this May 7, 2024
@cratelyn cratelyn marked this pull request as ready for review May 7, 2024 22:28
hdevalence
hdevalence approved these changes May 8, 2024
View reviewed changes
Copy link
Member

@hdevalence hdevalence left a comment

Choose a reason for hiding this comment

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

LGTM!

@cratelyn @hdevalence
view: 🔎 address review comments
6189d37 
#4349 (comment)
#4349 (comment)

Co-Authored-By: Henry de Valence <hdevalence@penumbralabs.xyz>
@cratelyn cratelyn mentioned this pull request May 8, 2024
Closed
@cratelyn cratelyn merged commit 0ece706 into main May 8, 2024
13 checks passed
@cratelyn cratelyn deleted the kate/penumbra-view-tracing-instrumentation branch May 8, 2024 01:12
cratelyn added a commit that referenced this pull request May 24, 2024
@cratelyn
app: 📺 test that Planner can spend notes via mock consensus
3d78c52 
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
@cratelyn cratelyn mentioned this pull request May 24, 2024
Merged
1 task
cratelyn added a commit that referenced this pull request May 29, 2024
@cratelyn
app: 📺 test that Planner can spend notes via mock consensus
0d82c26 
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
cratelyn added a commit that referenced this pull request May 30, 2024
@cratelyn
app: 📺 test that Planner can spend notes via mock consensus
4cdeb6f 
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
cratelyn added a commit that referenced this pull request May 31, 2024
@cratelyn
app: 📺 test that Planner can spend notes via mock consensus
2b60d55 
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
cratelyn added a commit that referenced this pull request May 31, 2024
@cratelyn
app: 📺 test that Planner can spend notes via mock consensus
1dab144 
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
cratelyn added a commit that referenced this pull request Jun 3, 2024
@cratelyn
app: 📺 test that Planner can spend notes via mock consensus
7521990 
see #3913.

this is the exciting culmination of work in pr's #3996, #4349, #4435,
 #4447, #4460, and #4468.

this adds a mock consensus test that spawns a gRPC server, creates a
view server, and then plans a transaction using the `Planner`. the
effects of this are then observed using the `ViewClient`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@hdevalence hdevalence hdevalence approved these changes

Assignees

@cratelyn cratelyn

Labels
A-node Area: System design and implementation for node software A-telemetry Area: Metrics, logging, and other observability-related features C-enhancement Category: an enhancement to the codebase
Projects
Penumbra
Status: Done
Milestone
Sprint 6
Development

Successfully merging this pull request may close these issues.
2 participants
@cratelyn @hdevalence