-
-
Notifications
You must be signed in to change notification settings - Fork 229
feat(metrics): Trace-connected Metrics (Analyzers) #4840
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
base: main
Are you sure you want to change the base?
Changes from all commits
6be1e01
e462457
6c5ca23
9485ca4
aa46acb
e9c349e
484f17f
e65db5f
de065c7
5228fbe
32caf06
98991a3
d730432
e7bb36f
d2a02e1
b87bc4d
4b4bc03
68a2c7a
9855d05
70a58d0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,3 @@ | ||
| ; Shipped analyzer releases | ||
| ; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md | ||
|
|
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,8 @@ | ||
| ; Unshipped analyzer release | ||
| ; https://github.com/dotnet/roslyn-analyzers/blob/main/src/Microsoft.CodeAnalysis.Analyzers/ReleaseTrackingAnalyzers.Help.md | ||
|
|
||
| ### New Rules | ||
|
|
||
| Rule ID | Category | Severity | Notes | ||
| --------|----------|----------|------- | ||
| SENTRY1001 | Support | Warning | TraceConnectedMetricsAnalyzer | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,113 @@ | ||
| using Microsoft.CodeAnalysis; | ||
| using Microsoft.CodeAnalysis.Diagnostics; | ||
| using Microsoft.CodeAnalysis.Operations; | ||
|
|
||
| namespace Sentry.Compiler.Extensions.Analyzers; | ||
|
|
||
| /// <summary> | ||
| /// Guide consumers to use the public API of <see href="https://develop.sentry.dev/sdk/telemetry/metrics/">Sentry Trace-connected Metrics</see> correctly. | ||
| /// </summary> | ||
| [DiagnosticAnalyzer(LanguageNames.CSharp)] | ||
| public sealed class TraceConnectedMetricsAnalyzer : DiagnosticAnalyzer | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: do we want to do this in general? This started off as a weekend-project for me. |
||
| { | ||
| private static readonly string Title = "Unsupported numeric type of Metric"; | ||
| private static readonly string MessageFormat = "{0} is unsupported type for Sentry Metrics. The only supported types are byte, short, int, long, float, and double."; | ||
| private static readonly string Description = "Integers should be a 64-bit signed integer, while doubles should be a 64-bit floating point number."; | ||
|
|
||
| private static readonly DiagnosticDescriptor Rule = new( | ||
| id: DiagnosticIds.Sentry1001, | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: CHANGELOG Should we mention this Analyzer, and Analyzer in general, in the CHANGELOG? |
||
| title: Title, | ||
| messageFormat: MessageFormat, | ||
| category: DiagnosticCategories.Support, | ||
| defaultSeverity: DiagnosticSeverity.Warning, | ||
| isEnabledByDefault: true, | ||
| description: Description, | ||
| helpLinkUri: null | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. question: docs Should we document the Analyzer? |
||
| ); | ||
|
|
||
| /// <inheritdoc /> | ||
| public override ImmutableArray<DiagnosticDescriptor> SupportedDiagnostics { get; } = ImmutableArray.Create(Rule); | ||
|
|
||
| /// <inheritdoc /> | ||
| public override void Initialize(AnalysisContext context) | ||
| { | ||
| context.ConfigureGeneratedCodeAnalysis(GeneratedCodeAnalysisFlags.None); | ||
| context.EnableConcurrentExecution(); | ||
|
|
||
| context.RegisterOperationAction(Execute, OperationKind.Invocation); | ||
| } | ||
|
|
||
| private static void Execute(OperationAnalysisContext context) | ||
| { | ||
| Debug.Assert(context.Operation.Language == LanguageNames.CSharp); | ||
| Debug.Assert(context.Operation.Kind is OperationKind.Invocation); | ||
|
|
||
| context.CancellationToken.ThrowIfCancellationRequested(); | ||
|
|
||
| if (context.Operation is not IInvocationOperation invocation) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var method = invocation.TargetMethod; | ||
| if (method.DeclaredAccessibility != Accessibility.Public || method.IsStatic || method.Parameters.Length == 0) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (!method.IsGenericMethod || method.Arity != 1 || method.TypeArguments.Length != 1) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (method.ContainingAssembly is null || method.ContainingAssembly.Name != "Sentry") | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (method.ContainingNamespace is null || method.ContainingNamespace.Name != "Sentry") | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| string fullyQualifiedMetadataName; | ||
| if (method.Name is "EmitCounter" or "EmitGauge" or "EmitDistribution") | ||
| { | ||
| fullyQualifiedMetadataName = "Sentry.SentryMetricEmitter"; | ||
| } | ||
| else if (method.Name is "TryGetValue") | ||
| { | ||
| fullyQualifiedMetadataName = "Sentry.SentryMetric"; | ||
| } | ||
| else | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var typeArgument = method.TypeArguments[0]; | ||
| if (typeArgument.SpecialType is SpecialType.System_Byte or SpecialType.System_Int16 or SpecialType.System_Int32 or SpecialType.System_Int64 or SpecialType.System_Single or SpecialType.System_Double) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (typeArgument is ITypeParameterSymbol) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var sentryType = context.Compilation.GetTypeByMetadataName(fullyQualifiedMetadataName); | ||
| if (sentryType is null) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| if (!SymbolEqualityComparer.Default.Equals(method.ContainingType, sentryType)) | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| var location = invocation.Syntax.GetLocation(); | ||
| var diagnostic = Diagnostic.Create(Rule, location, typeArgument.ToDisplayString(SymbolDisplayFormats.FullNameFormat)); | ||
| context.ReportDiagnostic(diagnostic); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| namespace Sentry.Compiler.Extensions; | ||
|
|
||
| internal static class DiagnosticCategories | ||
| { | ||
| internal const string Support = nameof(Support); | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,6 @@ | ||
| namespace Sentry.Compiler.Extensions; | ||
|
|
||
| internal static class DiagnosticIds | ||
| { | ||
| internal const string Sentry1001 = "SENTRY1001"; | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -15,11 +15,36 @@ | |
| <PrivateAssets>all</PrivateAssets> | ||
| <IncludeAssets>runtime; build; native; contentfiles; analyzers; buildtransitive</IncludeAssets> | ||
| </PackageReference> | ||
| <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.3.1" PrivateAssets="all"/> | ||
| <PackageReference Include="Microsoft.CodeAnalysis.CSharp" Version="4.3.1" PrivateAssets="all" /> | ||
| </ItemGroup> | ||
|
|
||
| <!-- | ||
| We use Simon Cropp's Polyfill source-only package to access APIs in lower targets. | ||
| https://github.com/SimonCropp/Polyfill | ||
| --> | ||
| <ItemGroup> | ||
| <PackageReference Include="Polyfill" Version="9.8.1" PrivateAssets="all" /> | ||
|
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. note: see also #4879 |
||
| </ItemGroup> | ||
| <!-- We currently don't require Polyfills for System.Memory. Ensure the feature is disabled and suppress the MSBuild Warning from Polyfill. --> | ||
| <Target Name="BeforePreparePolyfill" BeforeTargets="PreparePolyfill"> | ||
| <PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0'"> | ||
| <PolyfillNoWarnIncorrectVersion>true</PolyfillNoWarnIncorrectVersion> | ||
| </PropertyGroup> | ||
| </Target> | ||
| <Target Name="AfterPreparePolyfill" AfterTargets="PreparePolyfill" DependsOnTargets="PreparePolyfill"> | ||
| <PropertyGroup Condition="'$(TargetFramework)' == 'netstandard2.0'"> | ||
| <DefineConstants>$([System.String]::Copy('$(DefineConstants)').Replace('FeatureMemory','').Replace(';;',';'))</DefineConstants> | ||
| </PropertyGroup> | ||
| </Target> | ||
|
|
||
| <ItemGroup> | ||
| <Using Remove="System.Text.Json" /> | ||
| <Using Remove="System.Text.Json.Serialization" /> | ||
| </ItemGroup> | ||
|
|
||
| <ItemGroup> | ||
| <AdditionalFiles Remove="AnalyzerReleases.Shipped.md" /> | ||
| <AdditionalFiles Remove="AnalyzerReleases.Unshipped.md" /> | ||
| </ItemGroup> | ||
|
|
||
| </Project> | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,12 @@ | ||
| using Microsoft.CodeAnalysis; | ||
|
|
||
| namespace Sentry.Compiler.Extensions; | ||
|
|
||
| internal static class SymbolDisplayFormats | ||
| { | ||
| internal static SymbolDisplayFormat FullNameFormat { get; } = new SymbolDisplayFormat( | ||
| globalNamespaceStyle: SymbolDisplayGlobalNamespaceStyle.Omitted, | ||
| typeQualificationStyle: SymbolDisplayTypeQualificationStyle.NameAndContainingTypesAndNamespaces, | ||
| genericsOptions: SymbolDisplayGenericsOptions.IncludeTypeParameters | ||
| ); | ||
| } |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
question: ID
I'm a a bit certain about the ID.
Also, I'm not sure if it should be two separate IDs, one for the
Emit*methods, and one forSentryMetric.TryGetValue<TValue>(out TValue value)method.