Skip to content
This repository was archived by the owner on Apr 23, 2021. It is now read-only.

+poc,instruments PoC of an Instruments.app instrument "tracing" (naive) #97

Merged
merged 1 commit into from
Aug 5, 2020

Conversation

ktoso
Copy link
Collaborator

@ktoso ktoso commented Jul 26, 2020

Since I had a moment over the holidays here I picked up a PoC i wanted to sanity check since a while.


Context: Since a few years Instruments allows for building custom instruments (yeah all the words are terribly overloaded here but I don't think we can do much about that to be honest). See session: https://developer.apple.com/wwdc18/410 Creating Custom Instruments - WWDC 2018


PoC / "Use Case" of using the tracing instruments API with signposts and collecting in instruments.

This is by no means super end to end perfect, but showcases somewhat the shape and how compatible (or not) the APIs are.

It's fairly okey, the fact that spans are classes means we can easily use them in signpost IDs which is good.

Some comments inline.


❗ THIS IS JUST A POC, NOT END GOAL OR "VERY REAL" TRACER ❗

Result:

@@ -36,7 +36,7 @@ public protocol Span {
var endTimestamp: Timestamp? { get }

/// The read-only `BaggageContext` of this `Span`, set when starting this `Span`.
var baggage: BaggageContext { get }
var context: BaggageContext { get }
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

IMHO these should be context because span.context rather than span.baggage.

The baggage word is only to be used to disambiguate in cases like context.context so this isn't one of those cases IMO. (rules we arrived at https://github.com/slashmo/gsoc-swift-baggage-context#existing-context-argument )

Copy link
Contributor

Choose a reason for hiding this comment

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

requires major version bump ;-)

Copy link
Owner

Choose a reason for hiding this comment

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

Agreed 👍

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The project was never tagged so we're not even a 0.0.0, a major bump from nothing still is nothing 😉

// ==== ----------------------------------------------------------------------------------------------------------------
// MARK: OSSignpost Tracing

@available(OSX 10.14, *)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

since signposts are only available since then.
(I may have gotten the availability wrong on other OSes)

import Instrumentation
import Foundation // string conversion for os_log seems to live here

#if os(macOS) || os(tvOS) || os(iOS) || os(watchOS)
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

this is an apple platforms only thing

@available(watchOS 3.0, *)
enum OSSignpostTracingKeys {
enum TraceParentIDs: BaggageContextKey {
typealias Value = [OSSignpostID]
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

not "really" used and I don't think there's a good way to make use of this info in instruments... though I may have to learn more about it. Not a huge goal of this PoC anyway.

<name>Konrad `ktoso` Malawski</name>
</owner>

<!-- Instruments Developer Help: https://help.apple.com/instruments/developer/mac/current/ -->
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

see here for docs how this format works, and also WWDC sessions on custom instruments:

</os-signpost-interval-schema>

<!-- MARK: Instrument Definitions -->
<instrument>
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

just a simple instrument that shows each span (grouped by operation name) as it's own row.

This is obviously would not scale all that well in reality, but it's good for the PoC.

@ktoso ktoso requested a review from slashmo July 27, 2020 02:03
@ktoso
Copy link
Collaborator Author

ktoso commented Aug 4, 2020

Can this be merged, it's just another showcase -- wdyt @slashmo ?

I'll rebase after review.

@pokryfka
Copy link
Contributor

pokryfka commented Aug 4, 2020

I think this would be very useful with NIO channels, to see in Xcode when exactly - including the order- each job is run.

#48

Comment on lines +52 to +67
public func startSpan(
named operationName: String,
context: BaggageContext,
ofKind kind: SpanKind,
at timestamp: Timestamp?
) -> Span {
OSSignpostSpan(
log: self.log,
named: operationName,
signpostName: self.signpostName,
context: context
// , kind ignored
// , timestamp ignored, we capture it automatically
)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

note that some tracer may choose not to expose setting the timestamp explicitly by user.

this is the case with XRay tracer as well, at the moment (I dont really see a use case for that);
arguably its easier to change XRay tracer than OSSignpostSpan ;-)

BTW this is not required by OT

Copy link
Owner

@slashmo slashmo left a comment

Choose a reason for hiding this comment

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

Great job with this. I just ran the Instrument and it looks very nice already 👍

@slashmo
Copy link
Owner

slashmo commented Aug 4, 2020

Can this be merged, it's just another showcase

@ktoso Yep, sorry for the delay

@ktoso ktoso merged commit 9adfd50 into slashmo:main Aug 5, 2020
@ktoso ktoso deleted the instrumentsapp branch August 5, 2020 07:47
@ktoso
Copy link
Collaborator Author

ktoso commented Aug 5, 2020

Thanks folks, just another fun example to include in the suite, merged then :shipit:

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants