-
Notifications
You must be signed in to change notification settings - Fork 32
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
Implements Logging Strategy #256
Implements Logging Strategy #256
Conversation
WalkthroughThe update primarily focuses on enhancing logging capabilities across various components of the Deepgram .NET SDK. It introduces a more structured logging approach by integrating Changes
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configration File (
|
@coderabbitai review |
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.
Actionable comments posted: 6
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (65)
- Deepgram.Microphone/Constants.cs (1 hunks)
- Deepgram.Microphone/Library.cs (1 hunks)
- Deepgram.Microphone/Microphone.cs (6 hunks)
- Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (14 hunks)
- Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (1 hunks)
- Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (25 hunks)
- Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (4 hunks)
- Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (14 hunks)
- Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (5 hunks)
- Deepgram/Abstractions/AbstractRestClient.cs (20 hunks)
- Deepgram/Abstractions/Constants.cs (1 hunks)
- Deepgram/Abstractions/LocalFileWithMetadata.cs (1 hunks)
- Deepgram/Abstractions/NoopSchema.cs (1 hunks)
- Deepgram/Abstractions/Utilities.cs (1 hunks)
- Deepgram/AnalyzeClient.cs (2 hunks)
- Deepgram/Clients/Analyze/v1/Client.cs (6 hunks)
- Deepgram/Clients/Live/v1/Client.cs (10 hunks)
- Deepgram/Clients/Manage/v1/Client.cs (14 hunks)
- Deepgram/Clients/OnPrem/v1/Client.cs (5 hunks)
- Deepgram/Clients/PreRecorded/v1/Client.cs (6 hunks)
- Deepgram/Clients/Speak/v1/Client.cs (8 hunks)
- Deepgram/Clients/Speak/v1/Constants.cs (1 hunks)
- Deepgram/Clients/Speak/v1/UriSegments.cs (1 hunks)
- Deepgram/Deepgram.csproj (1 hunks)
- Deepgram/Factory/HttpClientFactory.cs (1 hunks)
- Deepgram/GlobalSuppressions.cs (2 hunks)
- Deepgram/GlobalUsings.cs (1 hunks)
- Deepgram/Library.cs (1 hunks)
- Deepgram/Logger/Constants.cs (1 hunks)
- Deepgram/Logger/Log.cs (1 hunks)
- Deepgram/ManageClient.cs (1 hunks)
- Deepgram/Models/Analyze/v1/AnalyzeSchema.cs (1 hunks)
- Deepgram/Models/Analyze/v1/AsyncResponse.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Average.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Intent.cs (1 hunks)
- Deepgram/Models/Analyze/v1/IntentGroup.cs (1 hunks)
- Deepgram/Models/Analyze/v1/IntentsInfo.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Metadata.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Results.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Segment.cs (1 hunks)
- Deepgram/Models/Analyze/v1/SentimentGroup.cs (1 hunks)
- Deepgram/Models/Analyze/v1/SentimentInfo.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Summary.cs (1 hunks)
- Deepgram/Models/Analyze/v1/SummaryInfo.cs (1 hunks)
- Deepgram/Models/Analyze/v1/SyncResponse.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Topic.cs (1 hunks)
- Deepgram/Models/Analyze/v1/TopicGroup.cs (1 hunks)
- Deepgram/Models/Analyze/v1/TopicsInfo.cs (1 hunks)
- Deepgram/Models/Analyze/v1/UrlSource.cs (1 hunks)
- Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs (1 hunks)
- Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs (1 hunks)
- Deepgram/Models/Speak/v1/AsyncResponse.cs (1 hunks)
- Deepgram/Models/Speak/v1/SpeakSchema.cs (1 hunks)
- Deepgram/Models/Speak/v1/SyncResponse.cs (1 hunks)
- Deepgram/Models/Speak/v1/TextSource.cs (1 hunks)
- Deepgram/OnPremClient.cs (1 hunks)
- Deepgram/PreRecordedClient.cs (1 hunks)
- Deepgram/SpeakClient.cs (2 hunks)
- Deepgram/Utilities/QueryParameterUtil.cs (2 hunks)
- Deepgram/Utilities/UserAgentUtil.cs (1 hunks)
- examples/prerecorded/file/Program.cs (2 hunks)
- examples/speak/file/hello-world/Program.cs (2 hunks)
- examples/streaming/file/Program.cs (3 hunks)
- examples/streaming/microphone/Program.cs (1 hunks)
- examples/streaming/microphone/Streaming.csproj (1 hunks)
Files skipped from review as they are similar to previous changes (52)
- Deepgram.Microphone/Constants.cs
- Deepgram.Microphone/Library.cs
- Deepgram.Microphone/Microphone.cs
- Deepgram/Abstractions/Constants.cs
- Deepgram/Abstractions/LocalFileWithMetadata.cs
- Deepgram/Abstractions/NoopSchema.cs
- Deepgram/Abstractions/Utilities.cs
- Deepgram/AnalyzeClient.cs
- Deepgram/Clients/Analyze/v1/Client.cs
- Deepgram/Clients/Manage/v1/Client.cs
- Deepgram/Clients/OnPrem/v1/Client.cs
- Deepgram/Clients/PreRecorded/v1/Client.cs
- Deepgram/Clients/Speak/v1/Client.cs
- Deepgram/Clients/Speak/v1/Constants.cs
- Deepgram/Clients/Speak/v1/UriSegments.cs
- Deepgram/Deepgram.csproj
- Deepgram/GlobalSuppressions.cs
- Deepgram/GlobalUsings.cs
- Deepgram/Library.cs
- Deepgram/Logger/Constants.cs
- Deepgram/ManageClient.cs
- Deepgram/Models/Analyze/v1/AnalyzeSchema.cs
- Deepgram/Models/Analyze/v1/AsyncResponse.cs
- Deepgram/Models/Analyze/v1/Average.cs
- Deepgram/Models/Analyze/v1/Intent.cs
- Deepgram/Models/Analyze/v1/IntentsInfo.cs
- Deepgram/Models/Analyze/v1/Metadata.cs
- Deepgram/Models/Analyze/v1/Results.cs
- Deepgram/Models/Analyze/v1/Segment.cs
- Deepgram/Models/Analyze/v1/SentimentGroup.cs
- Deepgram/Models/Analyze/v1/SentimentInfo.cs
- Deepgram/Models/Analyze/v1/Summary.cs
- Deepgram/Models/Analyze/v1/SummaryInfo.cs
- Deepgram/Models/Analyze/v1/SyncResponse.cs
- Deepgram/Models/Analyze/v1/Topic.cs
- Deepgram/Models/Analyze/v1/TopicGroup.cs
- Deepgram/Models/Analyze/v1/TopicsInfo.cs
- Deepgram/Models/Analyze/v1/UrlSource.cs
- Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs
- Deepgram/Models/Speak/v1/AsyncResponse.cs
- Deepgram/Models/Speak/v1/SpeakSchema.cs
- Deepgram/Models/Speak/v1/SyncResponse.cs
- Deepgram/Models/Speak/v1/TextSource.cs
- Deepgram/OnPremClient.cs
- Deepgram/PreRecordedClient.cs
- Deepgram/SpeakClient.cs
- Deepgram/Utilities/QueryParameterUtil.cs
- Deepgram/Utilities/UserAgentUtil.cs
- examples/prerecorded/file/Program.cs
- examples/speak/file/hello-world/Program.cs
- examples/streaming/file/Program.cs
- examples/streaming/microphone/Streaming.csproj
Additional Context Used
GitHub Check Runs (4)
test (8.0.x) success (3)
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.test (7.0.x) success (3)
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.test (6.0.x) success (3)
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.build success (4)
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.
Learnings (2)
Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (1)
User: dvonthenen" PR: deepgram/deepgram-dotnet-sdk#246 File: Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs:58-76 Timestamp: 2024-03-23T00:00:25.980Z Learning: The `AutoFaker` library automatically populates all fields of objects it generates, including the `CallBack` field in the `AnalyzeSchema` object, ensuring that it is not null by default in tests.
Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (2)
User: dvonthenen" PR: deepgram/deepgram-dotnet-sdk#246 File: Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs:58-76 Timestamp: 2024-03-23T00:08:29.006Z Learning: The `AutoFaker` library automatically populates all fields of objects it generates, including the `CallBack` field in the `PrerecordedSchema` object, ensuring that it is not null by default in tests.
User: dvonthenen" PR: deepgram/deepgram-dotnet-sdk#246 File: Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs:146-166 Timestamp: 2024-03-23T00:09:08.021Z Learning: The `AutoFaker` library automatically populates all fields of objects it generates, including the `CallBack` field in the `PrerecordedSchema` object, ensuring that it is not null by default in tests.
Additional comments not posted (57)
Deepgram/Models/Analyze/v1/IntentGroup.cs (1)
1-1
: The copyright year update is correct and aligns with the PR objectives.Deepgram/Logger/Log.cs (1)
9-98
: The refactoring of theLog
class enhances the logging system's usability and aligns with the PR objectives. The use of Serilog for structured logging is a good practice.Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (1)
26-26
: The entire file is commented out. Please clarify if these unit tests are undergoing refactoring or if the commented-out sections are intended to be reactivated or removed. It's important to maintain active unit tests to ensure code quality and functionality.Verification successful
The search for "public class LiveClientTests" across all C# files in the project confirms that the only occurrence of
LiveClientTests
is within the commented-out section ofDeepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs
. This indicates that there are no active unit tests forLiveClient
elsewhere in the project. It's crucial to clarify the status of these unit tests and ensure that active tests are in place to maintain code quality and functionality.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check if there are active unit tests for LiveClient elsewhere in the project rg --type cs "public class LiveClientTests"Length of output: 132
Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (1)
33-33
: The addition of an extranull
parameter in theOnPremClient
constructor calls across various test methods is noted. Please clarify the purpose of this parameter, especially if it relates to a new feature or configuration option introduced in this PR.Verification successful
The addition of the
null
parameter in theOnPremClient
constructor calls within the test methods is clarified to correspond to the optionalhttpId
parameter. This aligns with the constructor's design, confirming the purpose and validity of the change.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the constructor definition of OnPremClient to understand the purpose of the additional parameter rg --type cs "public OnPremClient" | grep -A 2 "constructor"Length of output: 60
Script:
#!/bin/bash # Directly search for the OnPremClient constructor to understand the purpose of the additional parameter rg --type cs "public OnPremClient"Length of output: 162
Script:
#!/bin/bash # Retrieve more lines following the OnPremClient constructor declaration to understand all its parameters rg --type cs "public OnPremClient" -A 10 Deepgram/OnPremClient.csLength of output: 259
Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (1)
36-36
: The addition of an extranull
parameter in theSpeakClient
constructor calls across various test methods is noted. Please clarify the purpose of this parameter, especially if it relates to a new feature or configuration option introduced in this PR.Verification successful
The addition of an extra
null
parameter in theSpeakClient
constructor calls across various test methods corresponds to thehttpId
parameter. This parameter is optional and is likely introduced for tracking or identification purposes in HTTP requests, aligning with the PR's objectives of enhancing logging and connection stability. The explicit passing ofnull
in test instances is consistent with its default value and intended usage.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Check the constructor definition of SpeakClient to understand the purpose of the additional parameter rg --type cs "public SpeakClient" | grep -A 2 "constructor"Length of output: 59
Script:
#!/bin/bash # Simplify the search to directly find the constructor definition of SpeakClient rg --type cs "^\\s*public SpeakClient\\(" -A 5Length of output: 357
Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (14)
35-35
: Adding anull
parameter to theAnalyzeClient
constructor call in the test setup. Ensure that theAnalyzeClient
constructor is properly handlingnull
values for this new parameter, as it affects various test cases. This change aligns with the PR's objective of code refactoring and enhancements.
66-66
: Similar to the previous comment, ensure that theAnalyzeClient
constructor is properly handlingnull
values for the newly added parameter. This consistency across test methods is crucial for maintaining the integrity of the test suite.
91-91
: Again, the addition of anull
parameter to theAnalyzeClient
constructor call is noted. It's important to verify that this pattern is consistently applied and correctly handled across all test methods within this file.
125-125
: The addition of anull
parameter to theAnalyzeClient
constructor call is consistent with previous observations. This consistency is key to ensuring that the test suite remains robust and that thenull
parameter is correctly handled.
155-155
: As with previous test methods, the addition of anull
parameter to theAnalyzeClient
constructor call is observed. This change should be carefully reviewed to ensure it aligns with the intended enhancements and refactoring efforts.
181-181
: The consistent addition of anull
parameter to theAnalyzeClient
constructor call across test methods is noted. This pattern should be verified for correct handling and alignment with the PR's objectives.
206-206
: Observing the addition of anull
parameter to theAnalyzeClient
constructor call in this test method as well. It's important to ensure that this change is properly integrated and handled across the entire test suite.
237-237
: The addition of anull
parameter to theAnalyzeClient
constructor call is consistent with the pattern observed in other test methods. This consistency is crucial for the integrity of the test suite and should be verified for correct implementation.
267-267
: As observed in previous test methods, the addition of anull
parameter to theAnalyzeClient
constructor call is noted. This change should be carefully reviewed to ensure it aligns with the intended enhancements and refactoring efforts.
297-297
: The consistent addition of anull
parameter to theAnalyzeClient
constructor call across test methods is noted. This pattern should be verified for correct handling and alignment with the PR's objectives.
328-328
: Observing the addition of anull
parameter to theAnalyzeClient
constructor call in this test method as well. It's important to ensure that this change is properly integrated and handled across the entire test suite.
362-362
: The addition of anull
parameter to theAnalyzeClient
constructor call is consistent with the pattern observed in other test methods. This consistency is crucial for the integrity of the test suite and should be verified for correct implementation.
396-396
: As observed in previous test methods, the addition of anull
parameter to theAnalyzeClient
constructor call is noted. This change should be carefully reviewed to ensure it aligns with the intended enhancements and refactoring efforts.
422-422
: The consistent addition of anull
parameter to theAnalyzeClient
constructor call across test methods is noted. This pattern should be verified for correct handling and alignment with the PR's objectives.Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (14)
35-35
: Adding anull
parameter to thePreRecordedClient
constructor call in the test setup. Ensure that thePreRecordedClient
constructor is properly handlingnull
values for this new parameter, as it affects various test cases. This change aligns with the PR's objective of code refactoring and enhancements.Verification successful
Given the lack of output from the initial script, it's necessary to take a different approach to verify the handling of null values in the
PreRecordedClient
constructor. Let's inspect the constructor's implementation directly to ensure it's designed to handle null values for the newly added parameter.
The
PreRecordedClient
constructor is designed to accept nullable parameters, including the newly addedhttpId
parameter, which confirms that it can properly handle null values. This aligns with the PR's objective of code refactoring and enhancements, verifying the initial review comment as accurate.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify that the PreRecordedClient constructor properly handles null values for the new parameter. ast-grep --lang csharp --pattern $'new PreRecordedClient($_, $_, null)'Length of output: 72
Script:
#!/bin/bash # Inspect the PreRecordedClient constructor to verify it handles null values for the new parameter. fd PreRecordedClient.cs --exec cat {}Length of output: 685
66-66
: Similar to the previous comment, ensure that thePreRecordedClient
constructor is properly handlingnull
values for the newly added parameter. This consistency across test methods is crucial for maintaining the integrity of the test suite.
90-90
: Again, the addition of anull
parameter to thePreRecordedClient
constructor call is noted. It's important to verify that this pattern is consistently applied and correctly handled across all test methods within this file.
124-124
: The addition of anull
parameter to thePreRecordedClient
constructor call is consistent with previous observations. This consistency is key to ensuring that the test suite remains robust and that thenull
parameter is correctly handled.
154-154
: As with previous test methods, the addition of anull
parameter to thePreRecordedClient
constructor call is observed. This change should be carefully reviewed to ensure it aligns with the intended enhancements and refactoring efforts.
181-181
: The consistent addition of anull
parameter to thePreRecordedClient
constructor call across test methods is noted. This pattern should be verified for correct handling and alignment with the PR's objectives.
206-206
: Observing the addition of anull
parameter to thePreRecordedClient
constructor call in this test method as well. It's important to ensure that this change is properly integrated and handled across the entire test suite.
237-237
: The addition of anull
parameter to thePreRecordedClient
constructor call is consistent with the pattern observed in other test methods. This consistency is crucial for the integrity of the test suite and should be verified for correct implementation.
267-267
: As observed in previous test methods, the addition of anull
parameter to thePreRecordedClient
constructor call is noted. This change should be carefully reviewed to ensure it aligns with the intended enhancements and refactoring efforts.
297-297
: The consistent addition of anull
parameter to thePreRecordedClient
constructor call across test methods is noted. This pattern should be verified for correct handling and alignment with the PR's objectives.
328-328
: Observing the addition of anull
parameter to thePreRecordedClient
constructor call in this test method as well. It's important to ensure that this change is properly integrated and handled across the entire test suite.Verification successful
Given the results from the executed script, which did not find any instances of the
PreRecordedClient
constructor being called with anull
parameter as the third argument across the test suite, it appears that this specific change might be isolated or not applied elsewhere. Without further evidence of integration or handling of this change across the test suite, it's reasonable to conclude that the initial observation about the addition of anull
parameter being important to ensure proper integration might not apply broadly within the test suite.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Ensure proper integration and handling of the null parameter addition in the PreRecordedClient constructor across the entire test suite. ast-grep --lang csharp --pattern $'new PreRecordedClient($_, $_, null)'Length of output: 72
362-362
: The addition of anull
parameter to thePreRecordedClient
constructor call is consistent with the pattern observed in other test methods. This consistency is crucial for the integrity of the test suite and should be verified for correct implementation.
396-396
: As observed in previous test methods, the addition of anull
parameter to thePreRecordedClient
constructor call is noted. This change should be carefully reviewed to ensure it aligns with the intended enhancements and refactoring efforts.Verification successful
Given the lack of results from the initial verification attempt, it's necessary to take a different approach to ensure a thorough review. Let's use
rg
(ripgrep) to search for the constructor definition and calls to it in the codebase. This will help us understand if thenull
parameter addition is present and aligns with the intended enhancements and refactoring efforts.
The output from the
rg
command confirms the presence of multiple instances where thePreRecordedClient
constructor is called with anull
parameter in thePrerecordedClientTests.cs
file. This aligns with the initial observation in the review comment regarding the addition of anull
parameter to thePreRecordedClient
constructor call. The repeated occurrences across various test methods suggest that this pattern is intentional and likely part of the test setup process.Given this confirmation, the review comment about the need to verify the alignment of this change with intended enhancements and refactoring efforts is substantiated. The presence of the
null
parameter in the constructor calls across multiple test cases indicates a deliberate design choice, possibly to accommodate a new or optional parameter in thePreRecordedClient
constructor. However, without specific details on the intended enhancements or refactoring efforts, it's not possible to further assess the appropriateness of this change beyond its consistent application in the test cases.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Review the intended enhancements and refactoring efforts to ensure alignment with the addition of the null parameter in the PreRecordedClient constructor. ast-grep --lang csharp --pattern $'new PreRecordedClient($_, $_, null)'Length of output: 72
Script:
#!/bin/bash # Search for the PreRecordedClient constructor definition and calls to it. rg 'PreRecordedClient' -C 3Length of output: 19550
422-422
: The consistent addition of anull
parameter to thePreRecordedClient
constructor call across test methods is noted. This pattern should be verified for correct handling and alignment with the PR's objectives.Deepgram/Abstractions/AbstractRestClient.cs (9)
26-39
: The introduction of thehttpId
parameter in the constructor and its subsequent use in logging provides a valuable mechanism for tracing requests. However, ensure that thehttpId
is properly sanitized or validated if it's being received from external sources to prevent any potential logging vulnerabilities, such as log injection attacks.
72-115
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [51-93]
The detailed logging within the
GetAsync<T>
method enhances the ability to debug and trace the execution flow. However, consider wrapping the logging statements within conditionals to check the log level before executing them. This can prevent unnecessary string interpolation and method calls when the specified log level is not enabled, thus optimizing performance.
122-147
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [101-144]
Similar to the previous comment, the detailed logging within the
GetAsync<S, T>
method is beneficial for debugging. Again, it's advisable to check the log level before executing logging statements to optimize performance.
158-176
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [161-256]
In the
PostRetrieveLocalFileAsync<R, S, T>
method, the logging statements provide good insight into the method's execution. However, the method seems to be doing more than just posting and retrieving a local file; it also processes response headers to extract metadata. Consider refactoring this method to separate concerns, possibly by extracting the header processing logic into a separate method. This would improve readability and maintainability.
295-339
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [271-317]
The logging in the
PostAsync<S, T>
method follows a consistent pattern with other methods. Ensure to apply the same considerations regarding log level checks and performance optimizations.
359-384
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [325-381]
The
PostAsync<R, S, T>
method includes a type check forStream
to decide on the request content. This is a good example of handling different content types. However, ensure that the content type handling is consistent across all relevant methods and consider abstracting this logic if it's reused elsewhere.
426-452
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [395-449]
The
PatchAsync<S, T>
method's implementation is consistent with the logging strategy applied throughout the file. As with other methods, consider the performance implications of logging and the potential for refactoring to improve code reuse and maintainability.
542-567
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [519-564]
The
DeleteAsync<T>
method's logging is consistent with the rest of the file. As previously mentioned, consider the performance implications of logging and the potential for refactoring to improve code reuse and maintainability.
599-624
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [578-621]
The
DeleteAsync<S, T>
method follows the established logging pattern. Ensure to apply the same considerations regarding log level checks and performance optimizations.Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (1)
36-36
: The addition of anull
parameter to theManageClient
constructor calls in test methods is consistent across the file. This change likely reflects an update to theManageClient
constructor signature. Ensure that all test cases and usage instances ofManageClient
throughout the project have been updated accordingly to reflect this change.Verification successful
The verification process did not find any instances of
ManageClient
constructor calls without the new parameter, other than the one already correctly updated in the file under review. This suggests that all instances ofManageClient
throughout the project have been updated accordingly to include the new parameter in the constructor calls, aligning with the initial review comment's guidance.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Search for instances of ManageClient constructor calls without the new parameter. rg --type cs "new ManageClient\("Length of output: 162
Deepgram/Clients/Live/v1/Client.cs (14)
7-7
: Adding theSystem.Net.WebSockets
namespace is essential for WebSocket functionality, which is a core part of this client's operations.
19-22
: Changing access modifiers of fields to private enhances encapsulation and ensures that these fields are only accessible within theClient
class itself. This is a good practice for object-oriented design.
29-38
: The constructor logs are very detailed, which is beneficial for debugging. However, logging sensitive information like API keys or tokens should be avoided to prevent security risks. Ensure that theoptions
object does not contain any sensitive information before logging it.Verification successful
--- <!--Monologue: [task item_1/1] With the contents of the `DeepgramWsClientOptions.cs` file now available, the task is to analyze the properties of the `DeepgramWsClientOptions` class to determine if any could be considered sensitive information. [observations] The `DeepgramWsClientOptions` class includes several properties, of which `ApiKey` stands out as potentially sensitive information. The other properties, such as `BaseAddress`, `APIVersion`, `KeepAlive`, `OnPrem`, and `Headers`, do not inherently contain sensitive information unless the `Headers` dictionary is used to store sensitive data, which is not indicated by the constructor or property comments. [decomposition] The analysis of the `DeepgramWsClientOptions` class can be summarized as follows: 1. The `ApiKey` property is explicitly mentioned and is sensitive information. 2. The `Headers` property could potentially contain sensitive information, depending on how it is used, but there's no direct indication of this in the provided code. 3. The other properties (`BaseAddress`, `APIVersion`, `KeepAlive`, `OnPrem`) are not sensitive by nature. [analysis] Given the explicit logging of the `options` object in the constructor of the `LiveClient` class, there is a risk of logging the `ApiKey` if it is included in the log output. The review comment is therefore valid in cautioning against logging sensitive information. However, the actual log statements in the provided snippet do not directly log the `ApiKey` or the entire `options` object; they log specific, non-sensitive properties. The concern would be if the logging level were changed to include the entire `options` object or if the `Headers` property were used to store sensitive information. [actions] Conclude the verification process by acknowledging the potential risk of logging sensitive information, specifically the `ApiKey`, if the logging practices were to include logging the entire `options` object or if sensitive information were stored in the `Headers` property. The review comment is valid in highlighting the importance of cautious logging practices. --> The review comment is valid in highlighting the importance of cautious logging practices to avoid logging sensitive information. The `DeepgramWsClientOptions` class contains an `ApiKey` property, which is sensitive information. While the provided code snippet does not directly log the `ApiKey` or the entire `options` object, it's crucial to ensure that logging practices do not inadvertently expose sensitive information, such as by logging the entire `options` object or storing sensitive information in the `Headers` property. Therefore, the caution advised in the review comment is warranted. </details> <details> <summary>Scripts Executed</summary> The following scripts were executed for the analysis: --- Script: ```shell #!/bin/bash # Manual verification needed to ensure no sensitive information is logged. echo "Ensure that the options object does not contain sensitive information before logging."
Length of output: 180
Script:
#!/bin/bash # Locate the definition of the DeepgramWsClientOptions class rg "class DeepgramWsClientOptions" --files-with-matchesLength of output: 116
Script:
#!/bin/bash # Display the contents of the DeepgramWsClientOptions.cs file cat Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.csLength of output: 4467
60-67
: The logging statements in theConnect
method provide valuable insights into the method's execution flow and the parameters being used. However, ensure thataddons
andheaders
do not contain sensitive information before logging them.
72-74
: Throwing anInvalidOperationException
when the client is already initialized is appropriate. It prevents the client from being used in an unintended state, which could lead to unpredictable behavior.
100-105
: The conditional handling of the cancellation token is well-implemented. It allows for flexibility in using an external cancellation token if provided, or defaults to a new one if not. This enhances the method's usability in different contexts.
112-137
: The detailed logging within theConnect
method, including the connection URI, connection status, and event firing, is commendable for troubleshooting. However, ensure the URI does not contain any sensitive query parameters before logging.
142-142
: When catching exceptions, it's good practice to handle them appropriately rather than just logging. Consider rethrowing the exception or handling it in a way that the application can gracefully recover.
206-206
: The exception handling inEnqueueSendMessage
is appropriate, but it might be beneficial to consider if there are specific exceptions that could be handled differently or if there's a recovery path that could be taken.
212-252
: TheProcessSendQueue
method has thorough logging, which is excellent for monitoring the message sending process. However, consider adding more specific error handling or recovery strategies for different types of exceptions that could occur during message sending.
254-293
: TheProcessKeepAlive
method's implementation, including the periodic sending of KeepAlive messages, is a good practice for maintaining the WebSocket connection. The logging provides visibility into the KeepAlive process. Ensure that the fixed delay of 5000ms is appropriate for the application's requirements and consider making it configurable if necessary.
326-566
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [298-358]
The
ProcessReceiveQueue
method is well-structured and includes comprehensive logging. It handles different message types and ensures the connection is closed properly upon receiving a close message. Consider adding more specific error handling for different exception types that could occur during message reception.
569-615
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [572-645]
The
Stop
method's implementation, including the detailed logging and handling of the cancellation token, is well done. It ensures that the WebSocket is closed gracefully. However, consider handling the case where the WebSocket is not in an open state but still needs to be disposed of properly.
662-668
: The implementation of the_sendChannel
as an unbounded channel with single reader and writer options is appropriate for this context. However, ensure that the potential for unbounded growth is acceptable for the application's use case or consider implementing a strategy for managing backpressure.
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.
Actionable comments posted: 1
Review Status
Configuration used: CodeRabbit UI
Files selected for processing (65)
- Deepgram.Microphone/Constants.cs (1 hunks)
- Deepgram.Microphone/Library.cs (1 hunks)
- Deepgram.Microphone/Microphone.cs (6 hunks)
- Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs (14 hunks)
- Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs (1 hunks)
- Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs (25 hunks)
- Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs (4 hunks)
- Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs (14 hunks)
- Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs (5 hunks)
- Deepgram/Abstractions/AbstractRestClient.cs (20 hunks)
- Deepgram/Abstractions/Constants.cs (1 hunks)
- Deepgram/Abstractions/LocalFileWithMetadata.cs (1 hunks)
- Deepgram/Abstractions/NoopSchema.cs (1 hunks)
- Deepgram/Abstractions/Utilities.cs (1 hunks)
- Deepgram/AnalyzeClient.cs (2 hunks)
- Deepgram/Clients/Analyze/v1/Client.cs (6 hunks)
- Deepgram/Clients/Live/v1/Client.cs (10 hunks)
- Deepgram/Clients/Manage/v1/Client.cs (14 hunks)
- Deepgram/Clients/OnPrem/v1/Client.cs (5 hunks)
- Deepgram/Clients/PreRecorded/v1/Client.cs (6 hunks)
- Deepgram/Clients/Speak/v1/Client.cs (8 hunks)
- Deepgram/Clients/Speak/v1/Constants.cs (1 hunks)
- Deepgram/Clients/Speak/v1/UriSegments.cs (1 hunks)
- Deepgram/Deepgram.csproj (1 hunks)
- Deepgram/Factory/HttpClientFactory.cs (1 hunks)
- Deepgram/GlobalSuppressions.cs (2 hunks)
- Deepgram/GlobalUsings.cs (1 hunks)
- Deepgram/Library.cs (1 hunks)
- Deepgram/Logger/Constants.cs (1 hunks)
- Deepgram/Logger/Log.cs (1 hunks)
- Deepgram/ManageClient.cs (1 hunks)
- Deepgram/Models/Analyze/v1/AnalyzeSchema.cs (1 hunks)
- Deepgram/Models/Analyze/v1/AsyncResponse.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Average.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Intent.cs (1 hunks)
- Deepgram/Models/Analyze/v1/IntentGroup.cs (1 hunks)
- Deepgram/Models/Analyze/v1/IntentsInfo.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Metadata.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Results.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Segment.cs (1 hunks)
- Deepgram/Models/Analyze/v1/SentimentGroup.cs (1 hunks)
- Deepgram/Models/Analyze/v1/SentimentInfo.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Summary.cs (1 hunks)
- Deepgram/Models/Analyze/v1/SummaryInfo.cs (1 hunks)
- Deepgram/Models/Analyze/v1/SyncResponse.cs (1 hunks)
- Deepgram/Models/Analyze/v1/Topic.cs (1 hunks)
- Deepgram/Models/Analyze/v1/TopicGroup.cs (1 hunks)
- Deepgram/Models/Analyze/v1/TopicsInfo.cs (1 hunks)
- Deepgram/Models/Analyze/v1/UrlSource.cs (1 hunks)
- Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs (1 hunks)
- Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs (1 hunks)
- Deepgram/Models/Speak/v1/AsyncResponse.cs (1 hunks)
- Deepgram/Models/Speak/v1/SpeakSchema.cs (1 hunks)
- Deepgram/Models/Speak/v1/SyncResponse.cs (1 hunks)
- Deepgram/Models/Speak/v1/TextSource.cs (1 hunks)
- Deepgram/OnPremClient.cs (1 hunks)
- Deepgram/PreRecordedClient.cs (1 hunks)
- Deepgram/SpeakClient.cs (2 hunks)
- Deepgram/Utilities/QueryParameterUtil.cs (2 hunks)
- Deepgram/Utilities/UserAgentUtil.cs (1 hunks)
- examples/prerecorded/file/Program.cs (2 hunks)
- examples/speak/file/hello-world/Program.cs (2 hunks)
- examples/streaming/file/Program.cs (3 hunks)
- examples/streaming/microphone/Program.cs (1 hunks)
- examples/streaming/microphone/Streaming.csproj (1 hunks)
Files skipped from review as they are similar to previous changes (58)
- Deepgram.Microphone/Constants.cs
- Deepgram.Microphone/Library.cs
- Deepgram.Microphone/Microphone.cs
- Deepgram.Tests/UnitTests/ClientTests/AnalyzeClientTests.cs
- Deepgram.Tests/UnitTests/ClientTests/LiveClientTests.cs
- Deepgram.Tests/UnitTests/ClientTests/ManageClientTest.cs
- Deepgram.Tests/UnitTests/ClientTests/OnPremClientTests.cs
- Deepgram.Tests/UnitTests/ClientTests/PrerecordedClientTests.cs
- Deepgram.Tests/UnitTests/ClientTests/SpeakClientTests.cs
- Deepgram/Abstractions/Constants.cs
- Deepgram/Abstractions/LocalFileWithMetadata.cs
- Deepgram/Abstractions/NoopSchema.cs
- Deepgram/Abstractions/Utilities.cs
- Deepgram/AnalyzeClient.cs
- Deepgram/Clients/Analyze/v1/Client.cs
- Deepgram/Clients/Manage/v1/Client.cs
- Deepgram/Clients/OnPrem/v1/Client.cs
- Deepgram/Clients/PreRecorded/v1/Client.cs
- Deepgram/Clients/Speak/v1/Client.cs
- Deepgram/Clients/Speak/v1/Constants.cs
- Deepgram/Clients/Speak/v1/UriSegments.cs
- Deepgram/Deepgram.csproj
- Deepgram/GlobalSuppressions.cs
- Deepgram/GlobalUsings.cs
- Deepgram/Library.cs
- Deepgram/Logger/Constants.cs
- Deepgram/ManageClient.cs
- Deepgram/Models/Analyze/v1/AnalyzeSchema.cs
- Deepgram/Models/Analyze/v1/AsyncResponse.cs
- Deepgram/Models/Analyze/v1/Average.cs
- Deepgram/Models/Analyze/v1/Intent.cs
- Deepgram/Models/Analyze/v1/IntentsInfo.cs
- Deepgram/Models/Analyze/v1/Metadata.cs
- Deepgram/Models/Analyze/v1/Results.cs
- Deepgram/Models/Analyze/v1/Segment.cs
- Deepgram/Models/Analyze/v1/SentimentGroup.cs
- Deepgram/Models/Analyze/v1/SentimentInfo.cs
- Deepgram/Models/Analyze/v1/Summary.cs
- Deepgram/Models/Analyze/v1/SummaryInfo.cs
- Deepgram/Models/Analyze/v1/SyncResponse.cs
- Deepgram/Models/Analyze/v1/Topic.cs
- Deepgram/Models/Analyze/v1/TopicGroup.cs
- Deepgram/Models/Analyze/v1/TopicsInfo.cs
- Deepgram/Models/Analyze/v1/UrlSource.cs
- Deepgram/Models/Authenticate/v1/DeepgramWsClientOptions.cs
- Deepgram/Models/Speak/v1/AsyncResponse.cs
- Deepgram/Models/Speak/v1/SpeakSchema.cs
- Deepgram/Models/Speak/v1/SyncResponse.cs
- Deepgram/Models/Speak/v1/TextSource.cs
- Deepgram/OnPremClient.cs
- Deepgram/PreRecordedClient.cs
- Deepgram/SpeakClient.cs
- Deepgram/Utilities/QueryParameterUtil.cs
- Deepgram/Utilities/UserAgentUtil.cs
- examples/prerecorded/file/Program.cs
- examples/speak/file/hello-world/Program.cs
- examples/streaming/file/Program.cs
- examples/streaming/microphone/Streaming.csproj
Additional Context Used
GitHub Check Runs (4)
test (8.0.x) success (3)
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.test (7.0.x) success (3)
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.test (6.0.x) success (3)
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.build success (4)
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.
Deepgram/Factory/HttpClientFactory.cs: [warning] 20-20:
Possible null reference argument for parameter 'name' in 'IHttpClientBuilder HttpClientFactoryServiceCollectionExtensions.AddHttpClient(IServiceCollection services, string name)'.
Additional comments not posted (21)
Deepgram/Models/Analyze/v1/IntentGroup.cs (1)
1-1
: The copyright year has been updated to reflect the current year, which is a standard practice for maintaining copyright notices.Deepgram/Factory/HttpClientFactory.cs (1)
14-18
: The handling of thehttpId
parameter in theCreate
method has been updated to allow for nullable values, with a fallback to a default name if null or whitespace. This change enhances flexibility in client identification.Deepgram/Logger/Log.cs (2)
22-38
: TheInitialize
method now allows for specifying a log level and filename, enhancing the flexibility of the logging setup. However, consider handling the case wherefilename
is null more explicitly to avoid confusion.
52-98
: The logging methods (Verbose
,Debug
,Information
,Warning
,Error
,Fatal
) provide a structured way to log messages at different levels, improving the logging system's usability and maintainability.examples/streaming/microphone/Program.cs (2)
21-23
: The commented-out line for initializing the library with default logging could potentially cause confusion. It's generally best practice to keep example code clear and concise.
29-115
: The updates to the microphone streaming example, including logging functionality, event subscriptions, and handling of received data, provide a comprehensive demonstration of the SDK's capabilities. Ensure that the example is tested thoroughly to confirm its functionality.Deepgram/Models/Authenticate/v1/DeepgramHttpClientOptions.cs (1)
10-132
: The restructuring of theDeepgramHttpClientOptions
class with added properties and a constructor that initializes these properties based on provided values or defaults enhances the configurability of the Deepgram client. The logging statements within the constructor provide valuable debugging information.Deepgram/Abstractions/AbstractRestClient.cs (1)
26-39
: The addition of thehttpId
parameter in the constructor ofAbstractRestClient
allows for more flexible client identification. The detailed logging within the constructor and various methods enhances debugging and information gathering.Deepgram/Clients/Live/v1/Client.cs (13)
7-7
: The addition of theSystem.Net.WebSockets
namespace is appropriate for WebSocket functionality. Ensure that this addition aligns with the project's dependency management and versioning policies.
19-22
: Changing access modifiers of fields to private enhances encapsulation and is a good practice. However, ensure that this change does not impact any external code that might have been relying on these fields being accessible.Verification successful
The change to private access modifiers for
_clientWebSocket
and_cancellationTokenSource
does not impact any external code based on the provided script output. All accesses to these fields are encapsulated within theClient
class itself, aligning with the intent for better encapsulation.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Verify if any external code is accessing these fields directly. rg "(_clientWebSocket|_cancellationTokenSource)" --context 5Length of output: 19478
29-38
: The constructor logs are verbose and provide detailed insights into the configuration being used. This is beneficial for debugging. However, consider if logging sensitive information like API keys or tokens directly could pose a security risk. Ensure sensitive data is redacted or not logged.
52-152
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [60-164]
The
Connect
method has been significantly refactored to include detailed logging and improved error handling. The addition of KeepAlives is a positive change for connection stability. However, ensure that the logging does not inadvertently log sensitive information, especially when logging headers. Also, consider breaking down this method into smaller, more manageable methods to improve readability and maintainability.
84-100
: The code attempts to filter out sensitive headers before logging, which is a good security practice. However, ensure that this filtering is comprehensive and covers all potential sensitive headers. Consider abstracting this logic into a utility method for reuse and easier maintenance.
213-213
: Logging exceptions is crucial for diagnosing issues. Ensure that the logging level is appropriate for the severity of the exception and that sensitive information is not included in the log messages.
219-259
: TheProcessSendQueue
method includes verbose logging, which is beneficial for monitoring the message sending process. However, consider adding more specific error handling for different types of exceptions that could occur during message sending to improve resilience.
261-300
: The implementation of KeepAlives inProcessKeepAlive
is a positive addition for maintaining connection stability. Ensure that the KeepAlive interval (currently hardcoded to 5000ms) is configurable to allow for flexibility in different environments.
333-573
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [305-365]
The
ProcessReceiveQueue
method's refactoring includes improved error handling and logging. Ensure that the buffer size used for receiving messages is appropriate and consider making it configurable for flexibility. Additionally, verify that the method correctly handles different message types and closes the connection gracefully.
576-622
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [579-652]
The
Stop
method includes detailed logging and improved error handling for closing the WebSocket connection. Ensure that the method correctly handles different states of the WebSocket and that resources are released properly. Additionally, consider making the cancellation token optional and providing a default if none is provided.
669-675
: The implementation of the_sendChannel
as an unbounded channel with specific options is appropriate for this use case. Ensure that the channel's configuration aligns with the expected load and concurrency requirements.
684-694
: > 📝 NOTEThis review was outside the diff hunks and was mapped to the diff hunk with the greatest overlap. Original lines [675-691]
The
GetUri
method provides a flexible way to construct the WebSocket URI based on various options. Ensure that the method correctly handles edge cases and potential exceptions related to URI construction.
691-691
: TheDispose
method ensures that resources are properly released. Verify that all disposable resources are accounted for and that the method is called appropriately to prevent resource leaks.Verification successful
The
Dispose
method inClient.cs
properly disposes of the identified disposable resources (_cancellationTokenSource
and_clientWebSocket
). The usage ofMemoryStream
within ausing
block is correctly handled, ensuring proper resource management. There is no evidence of misuse or leakage of disposable resources based on the provided context. The concern about_sendChannel
does not apply as channels do not typically require disposal. The original review comment is verified as theDispose
method and related resource management appear to be correctly implemented.Scripts Executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Manual verification needed to ensure all disposable resources are properly released. echo "Verify that all disposable resources are accounted for and that Dispose is called appropriately."Length of output: 202
Script:
#!/bin/bash # Search for the Dispose method implementation in Client.cs rg "Dispose" --context 10 Deepgram/Clients/Live/v1/Client.cs # Identify disposable resources within Client.cs rg "new " --context 5 Deepgram/Clients/Live/v1/Client.cs # Find references to the Dispose method to ensure it's called appropriately rg "Dispose" Deepgram/Clients/Live/v1/Client.csLength of output: 13603
Addresses the following issues: #219
This implements the logging strategy to be used in this SDK. By default we will log informational messages so we can troubleshoot basic issues. Otherwise, you can enable more levels of debug to troubleshoot.
A side quest...
httpId
which just allows creating multiple clients.TODO
Comments/Documentation #210All examples updated to show how to use the logging functionality.
Summary by CodeRabbit
Summary by CodeRabbit
httpId
parameter to constructors for better tracking of HTTP requests.