fix(instrumentation-redis-4): avoid diag.error spam when configured client URL is the empty string (#2399)

Issue #2389 showed that this instrumentation would crash on a Redis
client configured with {url: ''} (an empty string). The crash was
fixed in #2397. This change avoids the once-crashy code, and hence
the diag.error spam, by using the same guard before attempting to parse
the configured client URL that the Redis client itself does, if (options?.url),

Refs: #2389

Author: trentm

plugins/node/opentelemetry-instrumentation-redis-4/src/utils.ts

@@ -46,7 +46,7 @@ function removeCredentialsFromDBConnectionStringAttribute(
   diag: DiagLogger,
   url?: unknown
 ): string | undefined {
-  if (typeof url !== 'string') {
+  if (typeof url !== 'string' || !url) {
     return;
   }
 

plugins/node/opentelemetry-instrumentation-redis-4/test/redis.test.ts

@@ -13,6 +13,8 @@
  * See the License for the specific language governing permissions and
  * limitations under the License.
  */
+
+import { diag, DiagLogLevel } from '@opentelemetry/api';
 import {
   getTestSpans,
   registerInstrumentationTesting,
@@ -288,6 +290,36 @@ describe('redis@^4.0.0', () => {
         expectAttributeConnString
       );
     });
+
+    it('with empty string for client URL, there is no crash and no diag.error', async () => {
+      // Note: This messily leaves the diag logger set for other tests.
+      const diagErrors = [] as any;
+      diag.setLogger(
+        {
+          verbose() {},
+          debug() {},
+          info() {},
+          warn() {},
+          error(...args) {
+            diagErrors.push(args);
+          },
+        },
+        DiagLogLevel.WARN
+      );
+
+      const newClient = createClient({ url: '' });
+      try {
+        await newClient.connect();
+      } catch (_err) {
+        // Ignore. If the test Redis is not at the default port we expect this
+        // to error.
+      }
+      await newClient.disconnect();
+
+      const [span] = getTestSpans();
+      assert.strictEqual(span.name, 'redis-connect');
+      assert.strictEqual(diagErrors.length, 0, "no diag.error's");
+    });
   });
 
   describe('multi (transactions) commands', () => {