Skip to content
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
39 changes: 38 additions & 1 deletion Package.swift
Original file line number Diff line number Diff line change
@@ -1,14 +1,24 @@
// swift-tools-version:5.9
import CompilerPluginSupport
import PackageDescription

let package = Package(
name: "swift-distributed-tracing",
platforms: [
.macOS(.v10_15),
.iOS(.v13),
.tvOS(.v13),
.watchOS(.v6),
.macCatalyst(.v13),
],
Copy link

Choose a reason for hiding this comment

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

This change is a semver major, are we actually intending to mint a new semver for this?

Copy link
Contributor Author

@porglezomp porglezomp Nov 25, 2024

Choose a reason for hiding this comment

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

This is required because swiftpm doesn't allow specifying this at any finer granularity than the whole package, and it can't handle the dependencies in only a single product.

I was unsure how strictly this would be a breaking change since there was no versions explicitly stated here, but wanted to discuss it.

I'm noticing that these are already the minimum platforms for swift-distributed-tracing-extras and so we could put these macros there instead of here without a semver major bump. I'd be happy with that as a place for this to live, Konrad just initially suggested here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Alternative PR over there: apple/swift-distributed-tracing-extras#42

Copy link

Choose a reason for hiding this comment

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

Yeah, unfortunately this is a breaking change because SwiftPM doesn’t take the minimum deployment targets into consideration when resolving versions. The result is broken builds, caused by picking up the new minima.

While we’re on the topic of breakage, Swift-syntax also represents a risk. Because of their versioning strategy it’s very easy to produce unsolveable build graphs, or forcing users to not be able to update past the current version. I’m generally nervous about putting this anywhere it might accidentally get picked up.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

For that at least—am I correct that package versions are only resolved for products that are dependent on, so that at least requires libraries to depend on the macros to get breakage? But yes I don't have any experience with releasing a package providing macros, I have no idea what is sufficient here. Just putting it in -extras? Putting it in a whole separate package?

Copy link
Member

Choose a reason for hiding this comment

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

I think extras might be fine -- this was replaced by apple/swift-distributed-tracing-extras#42

products: [
.library(name: "Instrumentation", targets: ["Instrumentation"]),
.library(name: "Tracing", targets: ["Tracing"]),
.library(name: "TracingMacros", targets: ["TracingMacros"]),
],
dependencies: [
.package(url: "https://github.com/apple/swift-service-context.git", from: "1.1.0")
.package(url: "https://github.com/apple/swift-service-context.git", from: "1.1.0"),
.package(url: "https://github.com/swiftlang/swift-syntax.git", from: "600.0.0-latest"),
],
targets: [
// ==== --------------------------------------------------------------------------------------------------------
Expand Down Expand Up @@ -43,5 +53,32 @@ let package = Package(
.target(name: "Tracing")
]
),

// ==== --------------------------------------------------------------------------------------------------------
// MARK: TracingMacros

.target(
name: "TracingMacros",
dependencies: [
.target(name: "Tracing"),
.target(name: "TracingMacrosImplementation"),
]
),
.macro(
name: "TracingMacrosImplementation",
dependencies: [
.product(name: "SwiftSyntaxMacros", package: "swift-syntax"),
.product(name: "SwiftCompilerPlugin", package: "swift-syntax"),
]
),
.testTarget(
name: "TracingMacrosTests",
dependencies: [
.target(name: "Tracing"),
.target(name: "TracingMacros"),
.target(name: "TracingMacrosImplementation"),
.product(name: "SwiftSyntaxMacrosTestSupport", package: "swift-syntax"),
]
),
]
)
38 changes: 38 additions & 0 deletions Sources/TracingMacros/Docs.docc/TracedOperationName.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,38 @@
# ``TracingMacros/TracedOperationName``

### Examples

The default behavior is to use the base name of the function, but you can
explicitly specify this as well. This creates a span named `"preheatOven"`:
```swift
@Traced(.baseName)
func preheatOven(temperature: Int)
```

You can request the full name of the function as the span name, this
creates a span named `"preheatOven(temperature:)"`:
```swift
@Traced(.fullName)
func preheatOven(temperature: Int)
```

And it is also initializable with a string literal for fully custom names,
this creates a span explicitly named `"preheat oven"`:
```swift
@Traced("preheat oven")
func preheatOven(temperature: Int)
```
And if you need to load an existing string value as a name, you can use
`.string(someString)` to adapt it.


## Topics

### Create Operation Names
- ``baseName``
- ``fullName``
- ``string(_:)``
- ``init(stringLiteral:)``

### Convert an Operation Name to a String
- ``operationName(baseName:fullName:)``
19 changes: 19 additions & 0 deletions Sources/TracingMacros/Docs.docc/index.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
# ``TracingMacros``

Macro helpers for Tracing.

## Overview

The TracingMacros module provides optional macros to make it easier to write traced code.

The ``Traced(_:context:ofKind:span:)`` macro lets you avoid the extra indentation that comes with
adopting traced code, and avoids having to keep the throws/try and async/await
in-sync with the body. You can just attach `@Traced` to a function and get
started.

## Topics

### Tracing functions
- ``Traced(_:context:ofKind:span:)``
- ``TracedOperationName``

97 changes: 97 additions & 0 deletions Sources/TracingMacros/TracedMacro.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,97 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift Distributed Tracing open source project
//
// Copyright (c) 2020-2024 Apple Inc. and the Swift Distributed Tracing project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of Swift Distributed Tracing project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
@_exported import ServiceContextModule
import Tracing

/// A span name for a traced operation, either derived from the function name or explicitly specified.
///
/// When using the ``Traced(_:context:ofKind:span:)`` macro, you can use this to customize the span name.
public struct TracedOperationName: ExpressibleByStringLiteral {
@usableFromInline
let value: Name

@usableFromInline
enum Name {
case baseName
case fullName
case string(String)
}

internal init(value: Name) {
self.value = value
}

/// Use a literal string as an operation name.
public init(stringLiteral: String) {
value = .string(stringLiteral)
}

/// Use the base name of the attached function.
///
/// For `func preheatOven(temperature: Int)` this is `"preheatOven"`.
public static let baseName = TracedOperationName(value: .baseName)

/// Use the full name of the attached function.
///
/// For `func preheatOven(temperature: Int)` this is `"preheatOven(temperature:)"`.
/// This is provided by the `#function` macro.
public static let fullName = TracedOperationName(value: .fullName)

/// Use an explicitly specified operation name.
public static func string(_ text: String) -> Self {
.init(value: .string(text))
}

/// Helper logic to support the `Traced` macro turning this operation name into a string.
/// Provided as an inference guide.
///
/// - Parameters:
/// - baseName: The value to use for the ``baseName`` case. Must be
/// specified explicitly because there's no equivalent of `#function`.
/// - fullName: The value to use for the ``fullName`` case.
@inlinable
@_documentation(visibility: internal)
public static func _getOperationName(_ name: Self, baseName: String, fullName: String = #function) -> String {
switch name.value {
case .baseName: baseName
case .fullName: fullName
case let .string(text): text
}
}
}

#if compiler(>=6.0)
/// Instrument a function to place the entire body inside a span.
///
/// This macro is equivalent to calling ``withSpan`` in the body, but saves an
/// indentation level and duplication. It introduces a `span` variable into the
/// body of the function which can be used to add attributes to the span.
///
/// Parameters are passed directly to ``withSpan`` where applicable,
/// and omitting the parameters from the macro omit them from the call, falling
/// back to the default.
///
/// - Parameters:
/// - operationName: The name of the operation being traced.
/// - context: The `ServiceContext` providing information on where to start the new ``Span``.
/// - kind: The ``SpanKind`` of the new ``Span``.
/// - spanName: The name of the span variable to introduce in the function. Pass `"_"` to omit it.
@attached(body)
Copy link
Member

Choose a reason for hiding this comment

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

Needs some docs.

The shape is good though! I like the spanName as well, that's quite niffty.

public macro Traced(
_ operationName: TracedOperationName = .baseName,
context: ServiceContext? = nil,
Copy link
Member

Choose a reason for hiding this comment

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

Hmm, technically is the default .current rather than nil?

Should the type be the @autoclosure () -> ServiceContext as well, so we avoid reading the local when tracing is disabled (not bootstrapped)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I'm not entirely sure about the answer here since this is my first major macro.

  • For the default: I'm using nil just to delegate the default handling to withSpan itself—we don't even construct the syntax to pass a parameter when it's omitted here. I don't fully understand how default parameters are even passed to the macro.
  • For the type: since this is a macro it's handled via syntax expansion, so the type is used for checking the interface, not as a runtime type. When the expression is interpolated into the call, I think it will end up becoming an autoclosure at the withSpan callsite?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I did some experimentation: The default value isn't passed to the macro at all, it seems to just be for documentation purposes?

Copy link
Contributor Author

@porglezomp porglezomp Nov 22, 2024

Choose a reason for hiding this comment

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

Since it seems to be only documentation—do we want to duplicate the default values from withSpan here, or cross-reference to say "look at the default values in the function documentation"? I can see arguments for both. I leaned towards omitting them here to not duplicate code across modules, but it's already duplicated across many overload so it's maybe worth it for the documentation's sake?

Copy link
Member

Choose a reason for hiding this comment

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

Oh good point about the autoclosure not mattering!

the nil maybe is fine after all, since it just means "don't pass anything" makes sense to me

For the docs: i think it's fine to not duplicate the docs, just a short note that it is passed to the withSpan will be enough

Copy link
Member

Choose a reason for hiding this comment

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

I see, yeah let's document to just look at the real method maybe

ofKind kind: SpanKind? = nil,
span spanName: String = "span"
) = #externalMacro(module: "TracingMacrosImplementation", type: "TracedMacro")
#endif
159 changes: 159 additions & 0 deletions Sources/TracingMacrosImplementation/TracedMacro.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,159 @@
//===----------------------------------------------------------------------===//
//
// This source file is part of the Swift Distributed Tracing open source project
//
// Copyright (c) 2020-2024 Apple Inc. and the Swift Distributed Tracing project authors
// Licensed under Apache License v2.0
//
// See LICENSE.txt for license information
// See CONTRIBUTORS.txt for the list of Swift Distributed Tracing project authors
//
// SPDX-License-Identifier: Apache-2.0
//
//===----------------------------------------------------------------------===//
import SwiftCompilerPlugin
import SwiftSyntax
import SwiftSyntaxBuilder
import SwiftSyntaxMacros

#if compiler(>=6.0)
public struct TracedMacro: BodyMacro {
public static func expansion(
of node: AttributeSyntax,
providingBodyFor declaration: some DeclSyntaxProtocol & WithOptionalCodeBlockSyntax,
in context: some MacroExpansionContext
) throws -> [CodeBlockItemSyntax] {
guard let function = declaration.as(FunctionDeclSyntax.self),
let body = function.body
else {
throw MacroExpansionErrorMessage("expected a function with a body")
}

// Construct a withSpan call matching the invocation of the @Traced macro
let (operationName, context, kind, spanName) = try extractArguments(from: node)
let baseNameExpr = ExprSyntax(StringLiteralExprSyntax(content: function.name.text))

var withSpanCall = FunctionCallExprSyntax("withSpan()" as ExprSyntax)!
let operationNameExpr: ExprSyntax
if let operationName {
if operationName.is(StringLiteralExprSyntax.self) {
operationNameExpr = operationName
} else {
operationNameExpr = "TracedOperationName._getOperationName(\(operationName), baseName: \(baseNameExpr))"
}
} else {
operationNameExpr = baseNameExpr
}
withSpanCall.arguments.append(LabeledExprSyntax(expression: operationNameExpr))
func appendComma() {
withSpanCall.arguments[withSpanCall.arguments.index(before: withSpanCall.arguments.endIndex)]
.trailingComma = .commaToken()
}
if let context {
appendComma()
withSpanCall.arguments.append(LabeledExprSyntax(label: "context", expression: context))
}
if let kind {
appendComma()
withSpanCall.arguments.append(LabeledExprSyntax(label: "ofKind", expression: kind))
}

// Introduce a span identifier in scope
var spanIdentifier: TokenSyntax = "span"
if let spanName {
spanIdentifier = .identifier(spanName)
}

// We want to explicitly specify the closure effect specifiers in order
// to avoid warnings about unused try/await expressions.
// We might as well explicitly specify the closure return type to help type inference.

let asyncClause = function.signature.effectSpecifiers?.asyncSpecifier
let returnClause = function.signature.returnClause
var throwsClause = function.signature.effectSpecifiers?.throwsClause
// You aren't allowed to apply "rethrows" as a closure effect
// specifier, so we have to convert this to a "throws" effect
if throwsClause?.throwsSpecifier.tokenKind == .keyword(.rethrows) {
throwsClause?.throwsSpecifier = .keyword(.throws)
}
var withSpanExpr: ExprSyntax = """
\(withSpanCall) { \(spanIdentifier) \(asyncClause)\(throwsClause)\(returnClause)in \(body.statements) }
"""

// Apply a try / await as necessary to adapt the withSpan expression

if function.signature.effectSpecifiers?.asyncSpecifier != nil {
withSpanExpr = "await \(withSpanExpr)"
}

if function.signature.effectSpecifiers?.throwsClause != nil {
withSpanExpr = "try \(withSpanExpr)"
}

return ["\(withSpanExpr)"]
}

static func extractArguments(
from node: AttributeSyntax
) throws -> (
operationName: ExprSyntax?,
context: ExprSyntax?,
kind: ExprSyntax?,
spanName: String?
) {
// If there are no arguments, we don't have to do any of these bindings
guard let arguments = node.arguments?.as(LabeledExprListSyntax.self) else {
return (nil, nil, nil, nil)
}

func getArgument(label: String) -> ExprSyntax? {
arguments.first(where: { $0.label?.identifier?.name == label })?.expression
}

// The operation name is the first argument if it's unlabeled
var operationName: ExprSyntax?
if let firstArgument = arguments.first, firstArgument.label == nil {
operationName = firstArgument.expression
}

let context = getArgument(label: "context")
let kind = getArgument(label: "ofKind")
var spanName: String?
let spanNameExpr = getArgument(label: "span")
if let spanNameExpr {
guard let stringLiteral = spanNameExpr.as(StringLiteralExprSyntax.self),
stringLiteral.segments.count == 1,
let segment = stringLiteral.segments.first,
let segmentText = segment.as(StringSegmentSyntax.self)
else {
throw MacroExpansionErrorMessage("span name must be a simple string literal")
}
let text = segmentText.content.text
let isValidIdentifier = DeclReferenceExprSyntax("\(raw: text)" as ExprSyntax)?.hasError == false
let isValidWildcard = text == "_"
guard isValidIdentifier || isValidWildcard else {
throw MacroExpansionErrorMessage("'\(text)' is not a valid parameter name")
}
spanName = text
}
return (
operationName: operationName,
context: context,
kind: kind,
spanName: spanName
)
}

}
#endif

@main
struct TracingMacroPlugin: CompilerPlugin {
#if compiler(>=6.0)
let providingMacros: [Macro.Type] = [
TracedMacro.self
]
#else
let providingMacros: [Macro.Type] = []
#endif
}
Loading