Skip to content

Enforcing the InstrumentationOption on all Tracer Operations#3620

Merged
iancooper merged 3 commits into
masterfrom
EnsureInstrumentationOptions
Jun 29, 2025
Merged

Enforcing the InstrumentationOption on all Tracer Operations#3620
iancooper merged 3 commits into
masterfrom
EnsureInstrumentationOptions

Conversation

@preardon

@preardon preardon commented Jun 5, 2025

Copy link
Copy Markdown
Member

enforcing the InstrumentationOption on all Tracer Operations

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Health Improved (1 files improve in Code Health)

Gates Failed
Enforce critical code health rules (1 file with Bumpy Road Ahead)
Enforce advisory code health rules (1 file with Complex Method)

Gates Passed
2 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce critical code health rules Violations Code Health Impact
BrighterTracer.cs 1 critical rule 8.82 → 9.22 Suppress
Enforce advisory code health rules Violations Code Health Impact
BrighterTracer.cs 1 advisory rule 8.82 → 9.22 Suppress
View Improvements
File Code Health Impact Categories Improved
BrighterTracer.cs 8.82 → 9.22 Code Duplication

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@preardon

preardon commented Jun 5, 2025

Copy link
Copy Markdown
Member Author

@iancooper can you please give me your initial thoughts on this approach, the biggest questions I have are around what we should be filtering with each Flag, should we be creating more flags?

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Code Health Improved (1 files improve in Code Health)

Gates Failed
Enforce critical code health rules (1 file with Bumpy Road Ahead)
Enforce advisory code health rules (1 file with Complex Method)

Gates Passed
2 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce critical code health rules Violations Code Health Impact
BrighterTracer.cs 1 critical rule 8.82 → 9.22 Suppress
Enforce advisory code health rules Violations Code Health Impact
BrighterTracer.cs 1 advisory rule 8.82 → 9.22 Suppress
View Improvements
File Code Health Impact Categories Improved
BrighterTracer.cs 8.82 → 9.22 Code Duplication

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@iancooper iancooper added 2 - In Progress feature request v10 .NET Pull requests that update .net code labels Jun 9, 2025
@iancooper

iancooper commented Jun 9, 2025

Copy link
Copy Markdown
Member

@iancooper can you please give me your initial thoughts on this approach, the biggest questions I have are around what we should be filtering with each Flag, should we be creating more flags?

I think the approach here is sensible. I would suspect we need "just enough" flags. So I think that database information is sensible as it drifts into another domain etc. I guess, we don't want to make configuring this a lot of work, but we do want to let you (a) reduce the noise/cost (b) remove things that might be insecure (i.e. message contents) from telemetry.

@preardon preardon force-pushed the EnsureInstrumentationOptions branch from c39ec95 to e6a8dbd Compare June 25, 2025 13:08

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Failed
Enforce critical code health rules (1 file with Low Cohesion, Bumpy Road Ahead)
Enforce advisory code health rules (1 file with Complex Method)

Gates Passed
2 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce critical code health rules Violations Code Health Impact
BrighterTracer.cs 2 critical rules 8.82 → 8.14 Suppress
Enforce advisory code health rules Violations Code Health Impact
BrighterTracer.cs 1 advisory rule 8.82 → 8.14 Suppress

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@preardon preardon force-pushed the EnsureInstrumentationOptions branch from e6a8dbd to 304ca32 Compare June 25, 2025 13:50

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Failed
Enforce critical code health rules (1 file with Low Cohesion, Bumpy Road Ahead)
Enforce advisory code health rules (1 file with Complex Method)

Gates Passed
2 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce critical code health rules Violations Code Health Impact
BrighterTracer.cs 2 critical rules 8.82 → 7.65 Suppress
Enforce advisory code health rules Violations Code Health Impact
BrighterTracer.cs 1 advisory rule 8.82 → 7.65 Suppress

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@preardon preardon force-pushed the EnsureInstrumentationOptions branch from 304ca32 to 71a3b16 Compare June 25, 2025 15:49

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Failed
Enforce critical code health rules (1 file with Low Cohesion, Bumpy Road Ahead)
Enforce advisory code health rules (1 file with Complex Method, Overall Code Complexity)

Gates Passed
2 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce critical code health rules Violations Code Health Impact
BrighterTracer.cs 2 critical rules 8.82 → 7.09 Suppress
Enforce advisory code health rules Violations Code Health Impact
BrighterTracer.cs 2 advisory rules 8.82 → 7.09 Suppress

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

var now = _timeProvider.GetUtcNow();

var tags = new ActivityTagsCollection
var tags = GetNewTagsCollection(options);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❌ New issue: Low Cohesion
This module has at least 3 different responsibilities amongst its 25 functions, threshold = 3

Suppress

if (!string.IsNullOrEmpty(info.serverAddress)) tags.Add(BrighterSemanticConventions.ServerAddress, info.serverAddress);
if (info.networkPeerPort != 0) tags.Add(BrighterSemanticConventions.NetworkPeerPort, info.networkPeerPort);
if (info.serverPort != 0) tags.Add(BrighterSemanticConventions.ServerPort, info.serverPort);
var tags = GetNewTagsCollection(options, BrighterSemanticConventions.DbInstrumentationDomain);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❌ Getting worse: Complex Method
CreateDbSpan increases in cyclomatic complexity from 11 to 12, threshold = 9

Suppress


/// <inheritdoc />
public Activity? CreateClaimCheckSpan(ClaimCheckSpanInfo info)
public Activity? CreateClaimCheckSpan(ClaimCheckSpanInfo info, InstrumentationOptions options = InstrumentationOptions.All)

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❌ New issue: Bumpy Road Ahead
CreateClaimCheckSpan has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function

Suppress

if (!string.IsNullOrEmpty(info.serverAddress)) tags.Add(BrighterSemanticConventions.ServerAddress, info.serverAddress);
if (info.networkPeerPort != 0) tags.Add(BrighterSemanticConventions.NetworkPeerPort, info.networkPeerPort);
if (info.serverPort != 0) tags.Add(BrighterSemanticConventions.ServerPort, info.serverPort);
var tags = GetNewTagsCollection(options, BrighterSemanticConventions.DbInstrumentationDomain);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❌ New issue: Bumpy Road Ahead
CreateDbSpan has 2 blocks with nested conditional logic. Any nesting of 2 or deeper is considered. Threshold is one single, nested block per function

Suppress

var now = _timeProvider.GetUtcNow();

var tags = new ActivityTagsCollection
var tags = GetNewTagsCollection(options);

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

❌ New issue: Overall Code Complexity
This module has a mean cyclomatic complexity of 4.28 across 25 functions. The mean complexity threshold is 4

Suppress

@preardon

Copy link
Copy Markdown
Member Author

This latest push should have the outlines of everything that needs to be done,

Some observations would be

  • Do we need Messaging & Requests?
  • Request Body is exclusively used for Bodies (MessageBody & RequestBody)
  • I've Added a Specific Brighter Instrumentation Option for Internal Operations, Thoughts?

@preardon preardon force-pushed the EnsureInstrumentationOptions branch from 71a3b16 to 98e7af0 Compare June 25, 2025 16:18

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Failed
Enforce critical code health rules (1 file with Low Cohesion, Bumpy Road Ahead)
Enforce advisory code health rules (1 file with Complex Method, Overall Code Complexity)

Gates Passed
2 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce critical code health rules Violations Code Health Impact
BrighterTracer.cs 2 critical rules 8.82 → 7.09 Suppress
Enforce advisory code health rules Violations Code Health Impact
BrighterTracer.cs 2 advisory rules 8.82 → 7.09 Suppress

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@preardon preardon marked this pull request as ready for review June 29, 2025 12:18

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Failed
Enforce critical code health rules (1 file with Low Cohesion, Bumpy Road Ahead)
Enforce advisory code health rules (1 file with Complex Method, Overall Code Complexity)

Gates Passed
2 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce critical code health rules Violations Code Health Impact
BrighterTracer.cs 2 critical rules 8.82 → 7.09 Suppress
Enforce advisory code health rules Violations Code Health Impact
BrighterTracer.cs 2 advisory rules 8.82 → 7.09 Suppress

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

preardon added 3 commits June 29, 2025 13:37
# Conflicts:
#	src/Paramore.Brighter/WrapPipeline.cs
#	src/Paramore.Brighter/WrapPipelineAsync.cs
#	tests/Paramore.Brighter.Core.Tests/Observability/Archive/When_archiving_from_the_outbox.cs
#	tests/Paramore.Brighter.Core.Tests/Observability/Archive/When_archiving_from_the_outbox_async.cs
#	tests/Paramore.Brighter.Core.Tests/Observability/CommandProcessor/Clear/When_Clearing_A_Message_A_Span_Is_Exported.cs
@preardon preardon force-pushed the EnsureInstrumentationOptions branch from 80cbe43 to 2fe73f7 Compare June 29, 2025 12:40

@codescene-delta-analysis codescene-delta-analysis Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Gates Failed
Enforce critical code health rules (1 file with Low Cohesion, Bumpy Road Ahead)
Enforce advisory code health rules (1 file with Complex Method, Overall Code Complexity)

Gates Passed
2 Quality Gates Passed

See analysis details in CodeScene

Reason for failure
Enforce critical code health rules Violations Code Health Impact
BrighterTracer.cs 2 critical rules 8.82 → 7.09 Suppress
Enforce advisory code health rules Violations Code Health Impact
BrighterTracer.cs 2 advisory rules 8.82 → 7.09 Suppress

Quality Gate Profile: Clean Code Collective
Want more control? Customize Code Health rules or catch issues early with our IDE extension and CLI tool.

@iancooper

Copy link
Copy Markdown
Member

A lot of work @preardon. Going to merge in, so that we can get ready to cut RC1

@iancooper iancooper merged commit 66d873b into master Jun 29, 2025
55 of 60 checks passed
@iancooper iancooper deleted the EnsureInstrumentationOptions branch August 13, 2025 21:06
DevJonny pushed a commit to DevJonny/Brighter that referenced this pull request Feb 28, 2026
…rCommand#3620)

* Plumb in Instrumentation Options

# Conflicts:
#	src/Paramore.Brighter/WrapPipeline.cs
#	src/Paramore.Brighter/WrapPipelineAsync.cs
#	tests/Paramore.Brighter.Core.Tests/Observability/Archive/When_archiving_from_the_outbox.cs
#	tests/Paramore.Brighter.Core.Tests/Observability/Archive/When_archiving_from_the_outbox_async.cs
#	tests/Paramore.Brighter.Core.Tests/Observability/CommandProcessor/Clear/When_Clearing_A_Message_A_Span_Is_Exported.cs

* Add XML Comments

* Fix rebase issues
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

3 - Done feature request .NET Pull requests that update .net code

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants