Skip to content

Add package metadata to observability scopes used by PackageContainers #5969

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

Conversation

abertelrud
Copy link
Contributor

@abertelrud abertelrud commented Dec 12, 2022

PackageContainer is a protocol that is implemented by various types involved in SwiftPM package resolution. They can emit messages but those messages don't get associated with the diagnostics.

There are currently three implementors of this protocol: FileSystemPackageContainer, SourceControlPackageContainer, and RegistryPackageContainer. All three just assign a given ObservabilityScope, and setting a package location metadata on that scope is a good way to make sure that all the diagnostics emitted to that scope get associated with the package.

This fix makes sure that diagnostics emitted through these scopes get properly associated with the package. This is particularly useful for manifest-related diagnostics, which inherit the scope used by the various kinds of PackageContainer.

Changes

  • add diagnosticsMetadata property in extension on PackageRerference, similar to the one on Package
  • use this property to associate package metadata with the ObservabilityScope
  • add this metadata with a couple of standalone emit calls in Workspace

rdar://103229985

PackageContainer is a protocol that is implemented by various types involved in SwiftPM package resolution.  They can emit messages but those messages don't get associated with the diagnostics.

There are currently three implementors of this protocol:  FileSystemPackageContainer, SourceControlPackageContainer, and RegistryPackageContainer.  All three just assign a given ObservabilityScope, and setting a package location metadata on that scope is a good way to make sure that all the diagnostics emitted to that scope get associated with the package.

This fix makes sure that diagnostics emitted through these scopes get properly associated with the package.  This is particularly useful for manifest-related diagnostics, which inherit the scope used by the various kinds of PackageContainer.

rdar://103229985
@abertelrud
Copy link
Contributor Author

Looking for the best unit tests to extend with additional checks.

@abertelrud abertelrud closed this Dec 12, 2022
@abertelrud abertelrud reopened this Dec 12, 2022
@abertelrud
Copy link
Contributor Author

@swift-ci please smoke test

@abertelrud abertelrud self-assigned this Dec 12, 2022
@abertelrud abertelrud merged commit f79acf3 into swiftlang:main Dec 14, 2022
@abertelrud abertelrud deleted the eng/set-package-diagnostics-metadata branch December 14, 2022 18:23
public var diagnosticsMetadata: ObservabilityMetadata {
return .packageMetadata(identity: self.identity, kind: self.kind)
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

could this be a non-public extension closer to the call site? seems fairly leaky as a public property of the model

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.

3 participants