[POC] logging integration libdatadog to tracer #947
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…or improved type safety and clarity
BenchmarksComparisonBenchmark execution time: 2025-03-26 09:40:54 Comparing candidate commit 28563b3 in PR branch Found 1 performance improvements and 0 performance regressions! Performance is the same for 51 metrics, 2 unstable metrics. scenario:tags/replace_trace_tags
CandidateCandidate benchmark detailsGroup 1
Group 2
Group 3
Group 4
Group 5
Group 6
Group 7
Group 8
Group 9
Group 10
Group 11
Group 12
Group 13
BaselineOmitted due to size. |
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## main #947 +/- ##
==========================================
+ Coverage 72.85% 72.89% +0.04%
==========================================
Files 334 335 +1
Lines 50974 51159 +185
==========================================
+ Hits 37135 37294 +159
- Misses 13839 13865 +26
🚀 New features to boost your workflow:
|
![datadog-datadog-prod-us1[bot]](https://avatars.githubusercontent.com/u/365230?s=60&v=4){kind=small}
examples/logging/Console/Program.cs
Outdated
| } | ||
|
|
||
| // Helper method to create CharSlice from string | ||
| private static unsafe CharSlice CreateCharSlice(string message) |
There was a problem hiding this comment.
⚪ Code Vulnerability
Suggested change
| private static unsafe CharSlice CreateCharSlice(string message) | |
| private static CharSlice CreateCharSlice(string message) |
do not use unsafe method or code (...read more)
Avoid unsafe code blocks as much as possible. While unsafe blocks provide access to some important features of the C# language, you need to avoid using them as much as possible. For example, unsafe code allows developers to use pointers, but pointers and pointers arithmetic can lead to critical security issues. Unsafe code should be avoided or at least clearly identified in a small scope.
Learn More
| private static IntPtr messagePtr; | ||
|
|
||
| [GlobalSetup] | ||
| public unsafe void Setup() |
There was a problem hiding this comment.
⚪ Code Vulnerability
Suggested change
| public unsafe void Setup() | |
| public void Setup() |
do not use unsafe method or code (...read more)
Avoid unsafe code blocks as much as possible. While unsafe blocks provide access to some important features of the C# language, you need to avoid using them as much as possible. For example, unsafe code allows developers to use pointers, but pointers and pointers arithmetic can lead to critical security issues. Unsafe code should be avoided or at least clearly identified in a small scope.
Learn More
examples/logging/Console/Program.cs
Outdated
| } | ||
|
|
||
| // Helper method to convert CharSlice to string | ||
| private static unsafe string GetStringFromCharSlice(CharSlice slice) |
There was a problem hiding this comment.
⚪ Code Vulnerability
Suggested change
| private static unsafe string GetStringFromCharSlice(CharSlice slice) | |
| private static string GetStringFromCharSlice(CharSlice slice) |
do not use unsafe method or code (...read more)
Avoid unsafe code blocks as much as possible. While unsafe blocks provide access to some important features of the C# language, you need to avoid using them as much as possible. For example, unsafe code allows developers to use pointers, but pointers and pointers arithmetic can lead to critical security issues. Unsafe code should be avoided or at least clearly identified in a small scope.
Learn More
examples/logging/Console/Program.cs
Outdated
| { | ||
| ddog_Error_drop(errorPtr); | ||
| } | ||
| throw new Exception("Failed to initialize logger"); |
There was a problem hiding this comment.
🟠 Code Quality Violation
do not throw generic exceptions (...read more)
Exceptions should be specific to the application to help points the exact issue. For these reasons, generic exceptions should not be used and we should instead favor specific exception types.
examples/logging/Console/Program.cs
Outdated
| } | ||
|
|
||
| // Helper method to create CharSlice from string | ||
| private static unsafe CharSlice CreateCharSlice(string message) |
There was a problem hiding this comment.
⚪ Code Vulnerability
Suggested change
| private static unsafe CharSlice CreateCharSlice(string message) | |
| private static CharSlice CreateCharSlice(string message) |
do not use unsafe method or code (...read more)
Avoid unsafe code blocks as much as possible. While unsafe blocks provide access to some important features of the C# language, you need to avoid using them as much as possible. For example, unsafe code allows developers to use pointers, but pointers and pointers arithmetic can lead to critical security issues. Unsafe code should be avoided or at least clearly identified in a small scope.
Learn More
|
|
||
| public override bool IsInvalid => handle == IntPtr.Zero; | ||
|
|
||
| protected override bool ReleaseHandle() |
There was a problem hiding this comment.
🔵 Code Quality Violation
View all suggested fixes
Suggested change
| protected override bool ReleaseHandle() | |
| private override bool ReleaseHandle() |
Suggested change
| protected override bool ReleaseHandle() | |
| public override bool ReleaseHandle() |
do not use protected modifier with sealed class (...read more)
While authorized by the compiler, protected visibility in sealed classes does not make sense as these classes cannot be inherited. Use public or private instead.
| { | ||
| var messageBytes = Message.AsSpan<byte>(); | ||
| var message = Encoding.UTF8.GetString(messageBytes); | ||
| return new Exception(message); |
There was a problem hiding this comment.
🟠 Code Quality Violation
Suggested change
| return new Exception(message); | |
| throw return new Exception(message); |
Exception should be thrown (...read more)
Exceptions should be thrown and not just created. An expression such as new Exception(...) does not throw the exception. You should use the keyword throw to throw the exception`.
Learn More
| } | ||
|
|
||
| // Main logger implementation | ||
| public sealed class DatadogLogger : ILogger, IDisposable |
There was a problem hiding this comment.
🟠 Code Quality Violation
This class is considered dead code because it has a private constructor that isn't instantiated within the class and no public static declarations. (...read more)
Classes with a private constructor can't be instantiated outside of the class itself. Because they are unreachable, they should be removed or made public.
An exception is made for classes that access their own constructors (like a singleton), and classes that derive from System.Runtime.InteropServices.SafeHandle.
| private static IntPtr messagePtr; | ||
|
|
||
| [GlobalSetup] | ||
| public unsafe void Setup() |
There was a problem hiding this comment.
⚪ Code Vulnerability
Suggested change
| public unsafe void Setup() | |
| public void Setup() |
do not use unsafe method or code (...read more)
Avoid unsafe code blocks as much as possible. While unsafe blocks provide access to some important features of the C# language, you need to avoid using them as much as possible. For example, unsafe code allows developers to use pointers, but pointers and pointers arithmetic can lead to critical security issues. Unsafe code should be avoided or at least clearly identified in a small scope.
Learn More
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
What does this PR do?
A brief description of the change being made with this pull request.
Motivation
What inspired you to submit this pull request?
Additional Notes
Anything else we should know when reviewing?
How to test the change?
Describe here in detail how the change can be validated.