Skip to content

Protect the connection string tags extractor from an invalid connection string#5956

Merged
tonyredondo merged 2 commits into
masterfrom
tony/fix-connectionstring-cache-issue
Aug 28, 2024
Merged

Protect the connection string tags extractor from an invalid connection string#5956
tonyredondo merged 2 commits into
masterfrom
tony/fix-connectionstring-cache-issue

Conversation

@tonyredondo

Copy link
Copy Markdown
Member

Summary of changes

This PR fixes an scenario when the connection string could not be parsed by avoiding the exception.

Reason for change

Seen in Error tracking:

image

Implementation details

Wrap the method in a try catch block, and return default on that invalid connection string.
The caller method already returns default in other cases.

Test coverage

This is an small fix, I didn't write a test case for this... yet.

Other details

@tonyredondo
tonyredondo requested a review from a team as a code owner August 28, 2024 10:54
@tonyredondo tonyredondo self-assigned this Aug 28, 2024
@github-actions github-actions Bot added the area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) label Aug 28, 2024
@datadog-ddstaging

datadog-ddstaging Bot commented Aug 28, 2024

Copy link
Copy Markdown

Datadog Report

Branch report: tony/fix-connectionstring-cache-issue
Commit report: 76f8db9
Test service: dd-trace-dotnet

✅ 0 Failed, 300951 Passed, 1584 Skipped, 11h 51m 59.76s Total Time

@andrewlock

andrewlock commented Aug 28, 2024

Copy link
Copy Markdown
Member

Execution-Time Benchmarks Report ⏱️

Execution-time results for samples comparing the following branches/commits:

Execution-time benchmarks measure the whole time it takes to execute a program. And are intended to measure the one-off costs. Cases where the execution time results for the PR are worse than latest master results are shown in red. The following thresholds were used for comparing the execution times:

  • Welch test with statistical test for significance of 5%
  • Only results indicating a difference greater than 5% and 5 ms are considered.

Note that these results are based on a single point-in-time result for each branch. For full results, see the dashboard.

Graphs show the p99 interval based on the mean and StdDev of the test run, as well as the mean value of the run (shown as a diamond below the graph).

@andrewlock andrewlock left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Awesome, thanks!

Comment thread tracer/src/Datadog.Trace/Util/DbCommandCache.cs Outdated
@tonyredondo
tonyredondo merged commit 5efa35e into master Aug 28, 2024
@tonyredondo
tonyredondo deleted the tony/fix-connectionstring-cache-issue branch August 28, 2024 13:50
@github-actions github-actions Bot added this to the vNext-v3 milestone Aug 28, 2024
tonyredondo added a commit that referenced this pull request Aug 28, 2024
…on string (#5956)

## Summary of changes

This PR fixes an scenario when the connection string could not be parsed
by avoiding the exception.

## Reason for change

Seen in Error tracking:

<img width="1127" alt="image"
src="https://github.com/user-attachments/assets/436de8fe-b6b8-43d7-a894-5554bb4e04eb">

## Implementation details

Wrap the method in a try catch block, and return default on that invalid
connection string.
The caller method already returns `default` in other cases.

## Test coverage

This is an small fix, I didn't write a test case for this... yet.

## Other details
<!-- Fixes #{issue} -->

<!-- ⚠️ Note: where possible, please obtain 2 approvals prior to
merging. Unless CODEOWNERS specifies otherwise, for external teams it is
typically best to have one review from a team member, and one review
from apm-dotnet. Trivial changes do not require 2 reviews. -->

---------

Co-authored-by: Andrew Lock <[email protected]>
tonyredondo added a commit that referenced this pull request Aug 29, 2024
…on string (#5956 -> v2) (#5960)

## Summary of changes

This PR fixes an scenario when the connection string could not be parsed
by avoiding the exception.

## Reason for change

Seen in Error tracking:

<img width="1127" alt="image"

src="https://github.com/user-attachments/assets/436de8fe-b6b8-43d7-a894-5554bb4e04eb">

## Implementation details

Wrap the method in a try catch block, and return default on that invalid
connection string.
The caller method already returns `default` in other cases.

## Test coverage

This is an small fix, I didn't write a test case for this... yet.

## Other details

Backport of #5956

Co-authored-by: Andrew Lock <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

area:tracer The core tracer library (Datadog.Trace, does not include OpenTracing, native code, or integrations) identified-by:telemetry

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants