Skip to content

Implement tracing in the resource registry - #14

Open
jasagredo wants to merge 2 commits into
mainfrom
js/tracing-resource-registry
Open

Implement tracing in the resource registry#14
jasagredo wants to merge 2 commits into
mainfrom
js/tracing-resource-registry

Conversation

@jasagredo

Copy link
Copy Markdown
Contributor

No description provided.

Comment thread resource-registry/src/Control/ResourceRegistry.hs
Comment thread resource-registry/src/Control/ResourceRegistry.hs Outdated
Comment thread resource-registry/src/Control/ResourceRegistry.hs Outdated
Comment on lines +472 to +475
data Release m = Release {
runRelease :: m Bool
, releaseTracingInfo :: (Tracer m (Trace m), ResourceId, RegistryLabel)
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why isn't ResourceId and RegistryLabel part of Resource?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Because the resource id and the registry label might be updated. If I have them here I can update them when transferring, otherwise I need to introduce a TVar on each resource

newtype Release m = Release (m Bool)
data Release m = Release {
runRelease :: m Bool
, releaseTracingInfo :: (Tracer m (Trace m), ResourceId, RegistryLabel)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Why are we threading the tracer like this? Why not thread the (registry, resource) pair around?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Same as below:

Then you cannot release a resource without its registry which will be annoying


releaseResource :: Resource m -> m Bool
releaseResource Resource{resourceRelease = Release f} = f
releaseResource :: Monad m => Resource m -> m Bool

@bladyjoker bladyjoker Feb 4, 2026

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

If you make this into releaseResource :: Monad m => Registry m -> Resource m -> m Bool you can use the tracer and the registry label from the registry itself?

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

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

Then you cannot release a resource without its registry which will be annoying

@bladyjoker bladyjoker left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looks great!

@jasagredo
jasagredo force-pushed the js/tracing-resource-registry branch from 9576092 to 280ca32 Compare February 4, 2026 12:49
@jasagredo
jasagredo force-pushed the js/tracing-resource-registry branch 4 times, most recently from 4fa2859 to 80da86a Compare February 6, 2026 13:11

@bladyjoker bladyjoker left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Looking forward to seeing metrics pop up in cardano-node! Let's open an issue in ouroboros-consensus to track this?

@jasagredo
jasagredo force-pushed the js/tracing-resource-registry branch from 80da86a to fd271cc Compare February 16, 2026 14:48
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.

2 participants