feat: Add instrumentation for MetricKit. #963
Conversation
beekhc
requested review from
ArielDemarco,
bryce-b,
nachoBonafonte and
vvydier
as code owners
|
I don't know why the macOS tests are failing. |
I downloaded all macOS SDKs from version API_AVAILABLE(ios(13.0), macos(10.15)) API_UNAVAILABLE(tvos, watchos)
@interface MXMetricPayload : NSObject <NSSecureCoding>is in macOS After that, in all the others, it looks like this: API_AVAILABLE(ios(13.0)) API_UNAVAILABLE(macos, tvos, watchos)
@interface MXMetricPayload : NSObject <NSSecureCoding>Maybe it's related to this thing mentioned in the MXMetricManager documentation: So, seems that unless you compile it using the latest versions of Xcode (26.0 and up), this problem will happen Edit: I'd suggest removing macOS support until we fully migrate to Xcode 26.0 in CI/CD |
|
|
||
| let namespacedAttrs: [String: AttributeValueConvertable] = [ | ||
| "hang_duration": $0.hangDuration, | ||
| "exception.stacktrace_json": stacktraceJson |
There was a problem hiding this comment.
Nit: Why do we have exception.stacktrace and exception.stacktrace_tree? From the semconv, seems that different formats are already implied, and we can just add the metric kit json format to the list https://opentelemetry.io/docs/specs/semconv/exceptions/exceptions-spans/#stacktrace-representation
|
|
||
| @available(iOS 13.0, macOS 12.0, macCatalyst 13.1, visionOS 1.0, *) | ||
| public func reportMetrics(payload: MXMetricPayload) { | ||
| let span = getMetricKitTracer().spanBuilder(spanName: "MXMetricPayload") |
There was a problem hiding this comment.
I think there was some debate about capturing MXMetric as a metric, span, or log. Could you document your decision to record this as a span?
With the way its setup now, seems that we can also extend this to support metrics in the future
| decoding: callStackTree.jsonRepresentation(), | ||
| as: UTF8.self | ||
| ) | ||
| namespacedAttrs["exception.stacktrace_json"] = stacktraceJson |
There was a problem hiding this comment.
Same question here about stacktrace_json
|
|
||
| // Standard OTel exception attributes - will be populated below | ||
| var otelType: String? | ||
| var otelMessage: String? |
There was a problem hiding this comment.
How good are signalDescription and machExceptionDescription at describing crashes, and allowing them to be grouped together by common root cause?
|
Left a bunch of questions. Thanks Bee! |
| @@ -0,0 +1,244 @@ | |||
| # MetricKit Instrumentation | |||
3 participants
This is proposed instrumentation for Apple's MetricKit. The technical details for the instrumentation are thoroughly documented in the README file.
I am sure some details of the proposal will be controversial. I'd encourage you to particularly pay attention to:
MXMetricPayloadwith attributes for all reported valuesMXDiagnosticPayloadMXMetricPayloadspan is a 24-hour span over the time the data was aggregatedMXHistograms are converted to approximate averagesOf course any other feedback would also be appreciated.