Skip to content

remove clock protocol, use autoclosure "now" instant #120

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

Merged
merged 1 commit into from
Apr 13, 2023
Merged

Conversation

ktoso
Copy link
Member

@ktoso ktoso commented Apr 13, 2023

Motivation:

we don't need the clock protocol, but we do need the default clock to provide an instant.

We also add time to recordError in this PR

Resolves #119
Resolves #108

@ktoso ktoso force-pushed the wip-instants branch 2 times, most recently from 150a26e to d816c20 Compare April 13, 2023 02:40
**Motivation:**

we don't need the clock protocol, but we do need the default clock to
provide an instant.

We also add time to recordError in this PR
@ktoso
Copy link
Member Author

ktoso commented Apr 13, 2023

We have discussed this one with @fabianfett yesterday at length, it'll work nicely

@ktoso ktoso requested review from fabianfett, yim-lee and slashmo April 13, 2023 02:59
@ktoso
Copy link
Member Author

ktoso commented Apr 13, 2023

I'll merge since I want to get the documentation / reference docs revived today, but post-merge review is welcome -- this mostly just swaps clock: some TracerClock for at instant: @autoclosure () -> some TracerInstant but we default impl using TracerClock so we can still mock it out globally if we wanted to 👍

@ktoso ktoso merged commit 30631dd into main Apr 13, 2023
@ktoso ktoso deleted the wip-instants branch April 13, 2023 03:10
@@ -84,19 +84,19 @@ public struct NoOpTracer: LegacyTracer {
}
}

public func end<Clock: TracerClock>(clock: Clock) {
public func end<Instant: TracerInstant>(at instant: Instant) {
Copy link
Member

Choose a reason for hiding this comment

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

do we need an @autoclosure here as well?

Copy link
Member Author

Choose a reason for hiding this comment

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

Oh excellent, thanks! Following up right away

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.

addEvent and recordError should take clock argument Drop TracerClock from start/withSpan APIs, but use internally
2 participants