fix(e2e) - Change event logging to opt in to specific events only. #1824
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
vinagesh
force-pushed
the
vinagesh/eventsource
branch
from
4260f1f to
f4a83ea
Compare
vinagesh
commented
Mar 11, 2021
| EnableEvents( | ||
| eventSource, | ||
| EventLevel.LogAlways | ||
| if (_eventFilters.Any(filter => eventSource.Name.StartsWith(filter, StringComparison.OrdinalIgnoreCase))) |
There was a problem hiding this comment.
Only subscribe to events in the filter list. I had to use startswith as we have many source names like: Microsoft-Azure-Devices-Service-Client-Enter, Microsoft-Azure-Devices-Service-Client-Exit. It will be hard to consolidate all of that. This seems specific enough even with the startswith.
vinagesh
commented
Mar 11, 2021
| if (_eventFilters.Any(ef => eventData.EventSource.Name.StartsWith(ef, StringComparison.Ordinal))) | ||
| { | ||
| string eventIdent; | ||
| string eventIdent; |
There was a problem hiding this comment.
We already subscribed to a select set of events so we don't need any filtering here.
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
vinagesh
requested review from
abhipsaMisra,
azabbasi,
barustum,
bikamani,
drwill-ms,
jamdavi and
timtay-microsoft
as code owners
vinagesh
force-pushed
the
vinagesh/eventsource
branch
from
f4a83ea to
6ed381f
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
| using System.Globalization; | ||
| using System.Linq; | ||
|
|
||
| namespace System.Diagnostics.Tracing | ||
| { | ||
| public sealed class ConsoleEventListener : EventListener | ||
| { | ||
| private readonly string[] _eventFilters; | ||
| private static readonly string[] _eventFilters = new string[] { "DotNetty-Default", "Microsoft-Azure-Devices" }; |
There was a problem hiding this comment.
One suggestion is to allow the console logger to be initialized with a list of event providers. The reason for this is, we recommend customers use this class to help them debug their test code, and they wouldn't want all of our sdk relevant event sources to be written by default.
There was a problem hiding this comment.
Doesn't that assume customers log to ETW too?
There was a problem hiding this comment.
Here is what I found on testing. OnEventSourceCreated get called even before we populate the _eventFilters in the constructor. As soon as the base class constructor is called (which is before the derived class constructor), looks like this callback starts getting called. It was extremely odd but happens always when I debug and place a breakpoint. Hence, I removed that. It is possible to miss subscribing to some if we go that route. It becomes misleading. I am open to other suggestions.
There was a problem hiding this comment.
I could add a comment to configure that value to whatever is needed, which will clarify how to use it.
abhipsaMisra
approved these changes
Mar 11, 2021
drwill-ms
approved these changes
Mar 11, 2021
vinagesh
force-pushed
the
vinagesh/eventsource
branch
from
6ed381f to
ac4ba3f
Compare
|
/azp run |
|
Azure Pipelines successfully started running 1 pipeline(s). |
vinagesh
added a commit
that referenced
this pull request
Mar 11, 2021
* fix: samples readme links were out-of-date (#1784) * fix: samples arguments and remove 1 more dead link (#1785) * fix(readme): Update the location of ConsoleEventListener in our readme * doc(service-client) - Updating readme (#1799) * fix(doc): Update amqp transport exception doc to have detailed description for quota exceeded error mapping * feature(device-client): Make the DeviceClient and ModuleClient extensible (#1802) * doc(service-client): Added extra comments to clarify true and false in dispose (#1805) * feature,fix (device-client) Handle Twin failures using Amqp (#1796) * fix(iot-service): Update xml comments for ServiceClient * fix(iot-device): Update MqttTransportHandler to not use SemaphoreSlim.WaitAsync(TimeSpan, CancellationToken) * fix(iot-device): Update dotnetty task calls to use ConfigureAwait(true) * fix(iot-device): Fix MqttTransportHandler to not await on user supplied C2D callback * IoTHub Exception for Get and Patch Twin failures (#1815) * fix(edge): UnixDomainSocketEndPoint is available in .NET 2.1 and greater (#1816) * UnixDomainSocketEndPoint has been standard since 2.1. Allowing later versions to use the correct class The edge client HSM provider uses UnixDomainSockets (UDS) for communication. Before .NET 2.1 to implement a Unix Socket you had to create your own class to do so. Since 2.1 there has been a native UnixDomainSocketEndPoint class in the runtime. In 2.1 and 3.1 there is no issue. However in 5.0 there are some changes to the way the Socket class handles the native UnixDomainSocketEndPoint class. I didn't dig down extremely deep, but I suspect it's due to the way the endpoint handles the SocketAddress and the string manipulation there seeing as how there is a specific implementation for Windows and for Unix. * feat(e2e) - Enabling soft delete when creating keyvaults (#1820) * fix(e2e) - Change event logging to opt in to specific events only. (#1824) * (service-client): Design for IoT hub AAD authentication * (service-client: Refactor and add implementation for token credential input) (#1781) * (service-client): Refactor and add sas credential (#1786) * (service-client): Add constructors in service client to accept aad and sas tokens. (#1787) * (service-client): Add constructors in registry manager to accept aad and sas tokens. (#1788) * (service-client): Add constructors in job client to accept aad and sas tokens. (#1789) * (service-client): Add constructors to accept aad and sas tokens for digital twins client. (#1790) * fix(service-client): Support for AzureSasCredential for a better user experience (#1797) * doc(service-client): Update readme about the differnt client and operations (#1798) * tests(service-client): E2E tests for aad auth on all our clients (#1800) * test(service-client): Adding e2e tests for sas credential auth for IoT hub. (#1806) * feature(service-client): Adding chaching for aad tokens. (#1807) * fix(service-client)- Add IoT hub token scope. (#1812) Co-authored-by: David R. Williamson <[email protected]> Co-authored-by: Abhipsa Misra <[email protected]> Co-authored-by: bikamani <[email protected]> Co-authored-by: jamdavi <[email protected]>
abhipsaMisra
pushed a commit
that referenced
this pull request
Mar 19, 2021
vinagesh
added a commit
that referenced
this pull request
Mar 22, 2021
* fix: samples readme links were out-of-date (#1784) * fix: samples arguments and remove 1 more dead link (#1785) * fix(readme): Update the location of ConsoleEventListener in our readme * doc(service-client) - Updating readme (#1799) * fix(doc): Update amqp transport exception doc to have detailed description for quota exceeded error mapping * feature(device-client): Make the DeviceClient and ModuleClient extensible (#1802) * doc(service-client): Added extra comments to clarify true and false in dispose (#1805) * feature,fix (device-client) Handle Twin failures using Amqp (#1796) * fix(iot-service): Update xml comments for ServiceClient * fix(iot-device): Update MqttTransportHandler to not use SemaphoreSlim.WaitAsync(TimeSpan, CancellationToken) * fix(iot-device): Update dotnetty task calls to use ConfigureAwait(true) * fix(iot-device): Fix MqttTransportHandler to not await on user supplied C2D callback * IoTHub Exception for Get and Patch Twin failures (#1815) * fix(edge): UnixDomainSocketEndPoint is available in .NET 2.1 and greater (#1816) * UnixDomainSocketEndPoint has been standard since 2.1. Allowing later versions to use the correct class The edge client HSM provider uses UnixDomainSockets (UDS) for communication. Before .NET 2.1 to implement a Unix Socket you had to create your own class to do so. Since 2.1 there has been a native UnixDomainSocketEndPoint class in the runtime. In 2.1 and 3.1 there is no issue. However in 5.0 there are some changes to the way the Socket class handles the native UnixDomainSocketEndPoint class. I didn't dig down extremely deep, but I suspect it's due to the way the endpoint handles the SocketAddress and the string manipulation there seeing as how there is a specific implementation for Windows and for Unix. * feat(e2e) - Enabling soft delete when creating keyvaults (#1820) * fix(e2e) - Change event logging to opt in to specific events only. (#1824) * (service-client): Design for IoT hub AAD authentication * (service-client: Refactor and add implementation for token credential input) (#1781) * (service-client): Refactor and add sas credential (#1786) * (service-client): Add constructors in service client to accept aad and sas tokens. (#1787) * (service-client): Add constructors in registry manager to accept aad and sas tokens. (#1788) * (service-client): Add constructors in job client to accept aad and sas tokens. (#1789) * (service-client): Add constructors to accept aad and sas tokens for digital twins client. (#1790) * fix(service-client): Support for AzureSasCredential for a better user experience (#1797) * doc(service-client): Update readme about the differnt client and operations (#1798) * tests(service-client): E2E tests for aad auth on all our clients (#1800) * test(service-client): Adding e2e tests for sas credential auth for IoT hub. (#1806) * feature(service-client): Adding chaching for aad tokens. (#1807) * fix(service-client)- Add IoT hub token scope. (#1812) Co-authored-by: David R. Williamson <[email protected]> Co-authored-by: Abhipsa Misra <[email protected]> Co-authored-by: bikamani <[email protected]> Co-authored-by: jamdavi <[email protected]>
vinagesh
added a commit
that referenced
this pull request
Mar 22, 2021
* fix: samples readme links were out-of-date (#1784) * fix: samples arguments and remove 1 more dead link (#1785) * fix(readme): Update the location of ConsoleEventListener in our readme * doc(service-client) - Updating readme (#1799) * fix(doc): Update amqp transport exception doc to have detailed description for quota exceeded error mapping * feature(device-client): Make the DeviceClient and ModuleClient extensible (#1802) * doc(service-client): Added extra comments to clarify true and false in dispose (#1805) * feature,fix (device-client) Handle Twin failures using Amqp (#1796) * fix(iot-service): Update xml comments for ServiceClient * fix(iot-device): Update MqttTransportHandler to not use SemaphoreSlim.WaitAsync(TimeSpan, CancellationToken) * fix(iot-device): Update dotnetty task calls to use ConfigureAwait(true) * fix(iot-device): Fix MqttTransportHandler to not await on user supplied C2D callback * IoTHub Exception for Get and Patch Twin failures (#1815) * fix(edge): UnixDomainSocketEndPoint is available in .NET 2.1 and greater (#1816) * UnixDomainSocketEndPoint has been standard since 2.1. Allowing later versions to use the correct class The edge client HSM provider uses UnixDomainSockets (UDS) for communication. Before .NET 2.1 to implement a Unix Socket you had to create your own class to do so. Since 2.1 there has been a native UnixDomainSocketEndPoint class in the runtime. In 2.1 and 3.1 there is no issue. However in 5.0 there are some changes to the way the Socket class handles the native UnixDomainSocketEndPoint class. I didn't dig down extremely deep, but I suspect it's due to the way the endpoint handles the SocketAddress and the string manipulation there seeing as how there is a specific implementation for Windows and for Unix. * feat(e2e) - Enabling soft delete when creating keyvaults (#1820) * fix(e2e) - Change event logging to opt in to specific events only. (#1824) * (service-client): Design for IoT hub AAD authentication * (service-client: Refactor and add implementation for token credential input) (#1781) * (service-client): Refactor and add sas credential (#1786) * (service-client): Add constructors in service client to accept aad and sas tokens. (#1787) * (service-client): Add constructors in registry manager to accept aad and sas tokens. (#1788) * (service-client): Add constructors in job client to accept aad and sas tokens. (#1789) * (service-client): Add constructors to accept aad and sas tokens for digital twins client. (#1790) * fix(service-client): Support for AzureSasCredential for a better user experience (#1797) * doc(service-client): Update readme about the differnt client and operations (#1798) * tests(service-client): E2E tests for aad auth on all our clients (#1800) * test(service-client): Adding e2e tests for sas credential auth for IoT hub. (#1806) * feature(service-client): Adding chaching for aad tokens. (#1807) * fix(service-client)- Add IoT hub token scope. (#1812) Co-authored-by: David R. Williamson <[email protected]> Co-authored-by: Abhipsa Misra <[email protected]> Co-authored-by: bikamani <[email protected]> Co-authored-by: jamdavi <[email protected]>
vinagesh
added a commit
that referenced
this pull request
Mar 22, 2021
vinagesh
added a commit
that referenced
this pull request
Mar 22, 2021
* fix: samples readme links were out-of-date (#1784) * fix: samples arguments and remove 1 more dead link (#1785) * fix(readme): Update the location of ConsoleEventListener in our readme * doc(service-client) - Updating readme (#1799) * fix(doc): Update amqp transport exception doc to have detailed description for quota exceeded error mapping * feature(device-client): Make the DeviceClient and ModuleClient extensible (#1802) * doc(service-client): Added extra comments to clarify true and false in dispose (#1805) * feature,fix (device-client) Handle Twin failures using Amqp (#1796) * fix(iot-service): Update xml comments for ServiceClient * fix(iot-device): Update MqttTransportHandler to not use SemaphoreSlim.WaitAsync(TimeSpan, CancellationToken) * fix(iot-device): Update dotnetty task calls to use ConfigureAwait(true) * fix(iot-device): Fix MqttTransportHandler to not await on user supplied C2D callback * IoTHub Exception for Get and Patch Twin failures (#1815) * fix(edge): UnixDomainSocketEndPoint is available in .NET 2.1 and greater (#1816) * UnixDomainSocketEndPoint has been standard since 2.1. Allowing later versions to use the correct class The edge client HSM provider uses UnixDomainSockets (UDS) for communication. Before .NET 2.1 to implement a Unix Socket you had to create your own class to do so. Since 2.1 there has been a native UnixDomainSocketEndPoint class in the runtime. In 2.1 and 3.1 there is no issue. However in 5.0 there are some changes to the way the Socket class handles the native UnixDomainSocketEndPoint class. I didn't dig down extremely deep, but I suspect it's due to the way the endpoint handles the SocketAddress and the string manipulation there seeing as how there is a specific implementation for Windows and for Unix. * feat(e2e) - Enabling soft delete when creating keyvaults (#1820) * fix(e2e) - Change event logging to opt in to specific events only. (#1824) * (service-client): Design for IoT hub AAD authentication * (service-client: Refactor and add implementation for token credential input) (#1781) * (service-client): Refactor and add sas credential (#1786) * (service-client): Add constructors in service client to accept aad and sas tokens. (#1787) * (service-client): Add constructors in registry manager to accept aad and sas tokens. (#1788) * (service-client): Add constructors in job client to accept aad and sas tokens. (#1789) * (service-client): Add constructors to accept aad and sas tokens for digital twins client. (#1790) * fix(service-client): Support for AzureSasCredential for a better user experience (#1797) * doc(service-client): Update readme about the differnt client and operations (#1798) * tests(service-client): E2E tests for aad auth on all our clients (#1800) * test(service-client): Adding e2e tests for sas credential auth for IoT hub. (#1806) * feature(service-client): Adding chaching for aad tokens. (#1807) * fix(service-client)- Add IoT hub token scope. (#1812) Co-authored-by: David R. Williamson <[email protected]> Co-authored-by: Abhipsa Misra <[email protected]> Co-authored-by: bikamani <[email protected]> Co-authored-by: jamdavi <[email protected]>
vinagesh
added a commit
that referenced
this pull request
Mar 23, 2021
* fix: samples readme links were out-of-date (#1784) * fix: samples arguments and remove 1 more dead link (#1785) * fix(readme): Update the location of ConsoleEventListener in our readme * doc(service-client) - Updating readme (#1799) * fix(doc): Update amqp transport exception doc to have detailed description for quota exceeded error mapping * feature(device-client): Make the DeviceClient and ModuleClient extensible (#1802) * doc(service-client): Added extra comments to clarify true and false in dispose (#1805) * feature,fix (device-client) Handle Twin failures using Amqp (#1796) * fix(iot-service): Update xml comments for ServiceClient * fix(iot-device): Update MqttTransportHandler to not use SemaphoreSlim.WaitAsync(TimeSpan, CancellationToken) * fix(iot-device): Update dotnetty task calls to use ConfigureAwait(true) * fix(iot-device): Fix MqttTransportHandler to not await on user supplied C2D callback * IoTHub Exception for Get and Patch Twin failures (#1815) * fix(edge): UnixDomainSocketEndPoint is available in .NET 2.1 and greater (#1816) * UnixDomainSocketEndPoint has been standard since 2.1. Allowing later versions to use the correct class The edge client HSM provider uses UnixDomainSockets (UDS) for communication. Before .NET 2.1 to implement a Unix Socket you had to create your own class to do so. Since 2.1 there has been a native UnixDomainSocketEndPoint class in the runtime. In 2.1 and 3.1 there is no issue. However in 5.0 there are some changes to the way the Socket class handles the native UnixDomainSocketEndPoint class. I didn't dig down extremely deep, but I suspect it's due to the way the endpoint handles the SocketAddress and the string manipulation there seeing as how there is a specific implementation for Windows and for Unix. * feat(e2e) - Enabling soft delete when creating keyvaults (#1820) * fix(e2e) - Change event logging to opt in to specific events only. (#1824) * (service-client): Design for IoT hub AAD authentication * (service-client: Refactor and add implementation for token credential input) (#1781) * (service-client): Refactor and add sas credential (#1786) * (service-client): Add constructors in service client to accept aad and sas tokens. (#1787) * (service-client): Add constructors in registry manager to accept aad and sas tokens. (#1788) * (service-client): Add constructors in job client to accept aad and sas tokens. (#1789) * (service-client): Add constructors to accept aad and sas tokens for digital twins client. (#1790) * fix(service-client): Support for AzureSasCredential for a better user experience (#1797) * doc(service-client): Update readme about the differnt client and operations (#1798) * tests(service-client): E2E tests for aad auth on all our clients (#1800) * test(service-client): Adding e2e tests for sas credential auth for IoT hub. (#1806) * feature(service-client): Adding chaching for aad tokens. (#1807) * fix(service-client)- Add IoT hub token scope. (#1812) Co-authored-by: David R. Williamson <[email protected]> Co-authored-by: Abhipsa Misra <[email protected]> Co-authored-by: bikamani <[email protected]> Co-authored-by: jamdavi <[email protected]>
vinagesh
added a commit
that referenced
this pull request
Mar 23, 2021
* fix: samples readme links were out-of-date (#1784) * fix: samples arguments and remove 1 more dead link (#1785) * fix(readme): Update the location of ConsoleEventListener in our readme * doc(service-client) - Updating readme (#1799) * fix(doc): Update amqp transport exception doc to have detailed description for quota exceeded error mapping * feature(device-client): Make the DeviceClient and ModuleClient extensible (#1802) * doc(service-client): Added extra comments to clarify true and false in dispose (#1805) * feature,fix (device-client) Handle Twin failures using Amqp (#1796) * fix(iot-service): Update xml comments for ServiceClient * fix(iot-device): Update MqttTransportHandler to not use SemaphoreSlim.WaitAsync(TimeSpan, CancellationToken) * fix(iot-device): Update dotnetty task calls to use ConfigureAwait(true) * fix(iot-device): Fix MqttTransportHandler to not await on user supplied C2D callback * IoTHub Exception for Get and Patch Twin failures (#1815) * fix(edge): UnixDomainSocketEndPoint is available in .NET 2.1 and greater (#1816) * UnixDomainSocketEndPoint has been standard since 2.1. Allowing later versions to use the correct class The edge client HSM provider uses UnixDomainSockets (UDS) for communication. Before .NET 2.1 to implement a Unix Socket you had to create your own class to do so. Since 2.1 there has been a native UnixDomainSocketEndPoint class in the runtime. In 2.1 and 3.1 there is no issue. However in 5.0 there are some changes to the way the Socket class handles the native UnixDomainSocketEndPoint class. I didn't dig down extremely deep, but I suspect it's due to the way the endpoint handles the SocketAddress and the string manipulation there seeing as how there is a specific implementation for Windows and for Unix. * feat(e2e) - Enabling soft delete when creating keyvaults (#1820) * fix(e2e) - Change event logging to opt in to specific events only. (#1824) * (service-client): Design for IoT hub AAD authentication * (service-client: Refactor and add implementation for token credential input) (#1781) * (service-client): Refactor and add sas credential (#1786) * (service-client): Add constructors in service client to accept aad and sas tokens. (#1787) * (service-client): Add constructors in registry manager to accept aad and sas tokens. (#1788) * (service-client): Add constructors in job client to accept aad and sas tokens. (#1789) * (service-client): Add constructors to accept aad and sas tokens for digital twins client. (#1790) * fix(service-client): Support for AzureSasCredential for a better user experience (#1797) * doc(service-client): Update readme about the differnt client and operations (#1798) * tests(service-client): E2E tests for aad auth on all our clients (#1800) * test(service-client): Adding e2e tests for sas credential auth for IoT hub. (#1806) * feature(service-client): Adding chaching for aad tokens. (#1807) * fix(service-client)- Add IoT hub token scope. (#1812) Co-authored-by: David R. Williamson <[email protected]> Co-authored-by: Abhipsa Misra <[email protected]> Co-authored-by: bikamani <[email protected]> Co-authored-by: jamdavi <[email protected]>
timstewartm
pushed a commit
to timstewartm/azure-iot-sdk-csharp
that referenced
this pull request
May 30, 2024
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.
Checklist
masterbranch.Description of the changes
This change ensures that our E2E tests only subscribe to a specific set of event sources. After this change is checked in, I will take the changes into the aad branch as well. Having this change in master directly is beneficial and can help us upgrade the storage sdk.