Azure / azure-iot-sdk-java

A Java SDK for connecting devices to Microsoft Azure IoT services
https://azure.github.io/azure-iot-sdk-java/
Other
197 stars 235 forks source link

[feature request] Reduce thread name length #1715

Closed gary-newbolt closed 1 year ago

gary-newbolt commented 1 year ago

Is your feature request related to a problem? Please describe. We are currently undertaking the process of upgrading the SDK used in our code for our embedded devices, to make sure the SDK is compatible with the new TLS certificate authority Azure services are migrating to. When using SDK v1.34.0 and above, the JRE used on our embedded devices produces a warning message, "OpenJDK Embedded Client VM warning: can't set thread new name", when the SDK attempts to set the name of a thread used in certain processes within the SDK. This warning does not apprear in any version prior to 1.34.0, and as I understand it, making the thread name more identifiable was the key feature of the 1.34.0 release and persists in the 2.x.x versions also.

The issue is occuring due to a thread name limit on the underlying OS of 100 chars. The OS is the Blackberry QNX Neutrino 7.0 platform, which I appreciate is not on your supported platform list, however, this OS is popular in the embedded device world. Our embedded devices are also a closed system of a third party, for which we write code to enhance functionality and we have no way to configure the underlying OS.

To confirm that this was the problem, I downloaded the SDK code, built a version of the jar locally, and used in a test project to confirm the error still persisted. Then built a version of the SDK reverting the change to the thread name made in 1.34.0. This resolved the issue. Various iterations confirmed that any name with more than 100 chars caused the warning message when run on our embedded devices and in turn this caused by the limit configured in the OS.

Whilst this does not cause our devices to stop communicating, since the message is generated by the JRE, the message fills up our logs, making debugging other issues difficult.

Describe the solution you'd like In my analysis, many instances of thread names generated by the SDK exceed 100 chars. This made up from the format described below:

As you can see, before any of the variable components of the composit string name are determined, the name is already 83 chars.

The solution I would like would be a reduced thread name string length limited to a length maximum length, as I would argue that a smaller length format for a thread name would not make a thread any less identifiable.

Describe alternatives you've considered Alternatively, a configuration option passed to the DeviceClient SDK entry point, to some way change the requirement/format of this would be useful.

Additional context

timtay-microsoft commented 1 year ago

This is a reasonable ask. We'll look into this when we get the chance.

gary-newbolt commented 1 year ago

Thanks that would be great!

I appreciate you have a great deal on, any prioritisation would be welcome though. At the moment it is a blocker for us in our efforts to migrate to using the new CA for Azure services.

timtay-microsoft commented 1 year ago

If you get the chance, feel free to leave your feedback on #1720 @gary-newbolt

We don't want to change the default behavior because some users may have a dependency on it, but I have provided ways to opt-out of these complex thread names and have provided a way to specify your own prefix and/or suffix if you want to manage these threads in some custom way

gary-newbolt commented 1 year ago

Thanks, the new options to opt out, and add a custom prefix and/or suffix is great.

I appreciate the need to keep default behaviour the same for users who might be have a dependency on it. Would you consider that the case for the adjusting the the connectionId element of the default thread name and would reducing this to the first 8 chars of the UUID be a damaging to the existing users? This would reduce all names by 28 chars which would suit my specific case but I think also help to make the thread name more identifiable for anyone tracing a particular thread.

timtay-microsoft commented 1 year ago

Would you consider that the case for the adjusting the the connectionId element of the default thread name and would reducing this to the first 8 chars of the UUID be a damaging to the existing users?

In general, I would like to make this change. However, we try to err on the side of assuming that a change like this would break someone.

With that in mind, is #1720 sufficient for your needs?

gary-newbolt commented 1 year ago

I understand. Thanks for considering it. Yes, the solution would be sufficient. Thanks again!

timtay-microsoft commented 1 year ago

Gotcha. We'll get this merged in soon and released soon after. I'll let you know when we do the release so you can try it out!

timtay-microsoft commented 1 year ago

The release is now complete. This feature has been added as of DeviceClient version 2.2.0. See release notes for additional details.

I'll go ahead and close this thread, but feel free to open a new thread if you encounter any further issues!