Fix populating module name in telemetry policy

sdk/azcore/CHANGELOG.md

@@ -12,6 +12,7 @@
 * Method `Span.End()` now requires an argument of type `*tracing.SpanEndOptions`.
 
 ### Bugs Fixed
+* Fixed an issue in `azcore.NewClient()` and `arm.NewClient()` that could cause an incorrect module name to be used in telemetry.
 
 ### Other Changes
 

sdk/azcore/arm/client.go

@@ -28,12 +28,13 @@ type Client struct {
 
 // NewClient creates a new Client instance with the provided values.
 // This client is intended to be used with Azure Resource Manager endpoints.
-// - clientName - the fully qualified name of the client ("package.Client"); this is used by the tracing provider when creating spans
+// - clientName - the fully qualified name of the client ("module/package.Client"); this is used by the telemetry policy and tracing provider.
+// if module and package are the same value, the "module/" prefix can be omitted.
 // - moduleVersion - the version of the containing module; used by the telemetry policy
 // - cred - the TokenCredential used to authenticate the request
 // - options - optional client configurations; pass nil to accept the default values
 func NewClient(clientName, moduleVersion string, cred azcore.TokenCredential, options *ClientOptions) (*Client, error) {
- pkg, err := shared.ExtractPackageName(clientName)
+ mod, client, err := shared.ExtractModuleName(clientName)
  if err != nil {
  return nil, err
  }
@@ -52,12 +53,12 @@ func NewClient(clientName, moduleVersion string, cred azcore.TokenCredential, op
  if c, ok := options.Cloud.Services[cloud.ResourceManager]; ok {
  ep = c.Endpoint
  }
- pl, err := armruntime.NewPipeline(pkg, moduleVersion, cred, runtime.PipelineOptions{}, options)
+ pl, err := armruntime.NewPipeline(mod, moduleVersion, cred, runtime.PipelineOptions{}, options)
  if err != nil {
  return nil, err
  }
 
- tr := options.TracingProvider.NewTracer(clientName, moduleVersion)
+ tr := options.TracingProvider.NewTracer(client, moduleVersion)
  return &Client{ep: ep, pl: pl, tr: tr}, nil
 }
 

sdk/azcore/core.go

@@ -76,12 +76,13 @@ type Client struct {
 }
 
 // NewClient creates a new Client instance with the provided values.
-// - clientName - the fully qualified name of the client ("package.Client"); this is used by the tracing provider when creating spans
+// - clientName - the fully qualified name of the client ("module/package.Client"); this is used by the telemetry policy and tracing provider.
+// if module and package are the same value, the "module/" prefix can be omitted.
 // - moduleVersion - the semantic version of the containing module; used by the telemetry policy
 // - plOpts - pipeline configuration options; can be the zero-value
 // - options - optional client configurations; pass nil to accept the default values
 func NewClient(clientName, moduleVersion string, plOpts runtime.PipelineOptions, options *ClientOptions) (*Client, error) {
- pkg, err := shared.ExtractPackageName(clientName)
+ mod, client, err := shared.ExtractModuleName(clientName)
  if err != nil {
  return nil, err
  }
@@ -96,9 +97,9 @@ func NewClient(clientName, moduleVersion string, plOpts runtime.PipelineOptions,
  }
  }
 
- pl := runtime.NewPipeline(pkg, moduleVersion, plOpts, options)
+ pl := runtime.NewPipeline(mod, moduleVersion, plOpts, options)
 
- tr := options.TracingProvider.NewTracer(clientName, moduleVersion)
+ tr := options.TracingProvider.NewTracer(client, moduleVersion)
  if tr.Enabled() && plOpts.TracingNamespace != "" {
  tr.SetAttributes(tracing.Attribute{Key: "az.namespace", Value: plOpts.TracingNamespace})
  }

sdk/azcore/internal/shared/shared.go

@@ -13,7 +13,6 @@ import (
  "reflect"
  "regexp"
  "strconv"
- "strings"
  "time"
 
  "github.com/Azure/azure-sdk-for-go/sdk/internal/errorinfo"
@@ -87,16 +86,28 @@ func ValidateModVer(moduleVersion string) error {
  return nil
 }
 
-// ExtractPackageName returns "package" from "package.Client".
+// ExtractModuleName returns "module", "package.Client" from "module/package.Client" or
+// "package", "package.Client" from "package.Client" when there's no "module/" prefix.
 // If clientName is malformed, an error is returned.
-func ExtractPackageName(clientName string) (string, error) {
- pkg, client, ok := strings.Cut(clientName, ".")
- if !ok {
- return "", fmt.Errorf("missing . in clientName %s", clientName)
- } else if pkg == "" || client == "" {
- return "", fmt.Errorf("malformed clientName %s", clientName)
+func ExtractModuleName(clientName string) (string, string, error) {
+ // uses unnamed capturing for "module", "package.Client", and "package"
+ regex, err := regexp.Compile(`^(?:([a-z0-9]+)/)?(([a-z0-9]+)\.(?:[A-Za-z0-9]+))$`)
+ if err != nil {
+ return "", "", err
  }
- return pkg, nil
+
+ matches := regex.FindStringSubmatch(clientName)
+ if len(matches) < 4 {
+ return "", "", fmt.Errorf("malformed clientName %s", clientName)
+ }
+
+ // the first match is the entire string, the second is "module", the third is
+ // "package.Client" and the fourth is "package".
+ // if there was no "module/" prefix, the second match will be the empty string
+ if matches[1] != "" {
+ return matches[1], matches[2], nil
+ }
+ return matches[3], matches[2], nil
 }
 
 // NonRetriableError marks the specified error as non-retriable.

sdk/azcore/internal/shared/shared_test.go

@@ -85,24 +85,54 @@ func TestValidateModVer(t *testing.T) {
  require.Error(t, ValidateModVer("v1.2"))
 }
 
-func TestExtractPackageName(t *testing.T) {
- pkg, err := ExtractPackageName("package.Client")
+func TestExtractModuleName(t *testing.T) {
+ mod, client, err := ExtractModuleName("module/package.Client")
  require.NoError(t, err)
- require.Equal(t, "package", pkg)
+ require.Equal(t, "module", mod)
+ require.Equal(t, "package.Client", client)
 
- pkg, err = ExtractPackageName("malformed")
+ mod, client, err = ExtractModuleName("malformed/")
  require.Error(t, err)
- require.Empty(t, pkg)
+ require.Empty(t, mod)
+ require.Empty(t, client)
 
- pkg, err = ExtractPackageName(".malformed")
+ mod, client, err = ExtractModuleName("malformed/malformed")
  require.Error(t, err)
- require.Empty(t, pkg)
+ require.Empty(t, mod)
+ require.Empty(t, client)
 
- pkg, err = ExtractPackageName("malformed.")
+ mod, client, err = ExtractModuleName("malformed/malformed.")
  require.Error(t, err)
- require.Empty(t, pkg)
+ require.Empty(t, mod)
+ require.Empty(t, client)
 
- pkg, err = ExtractPackageName("")
+ mod, client, err = ExtractModuleName("malformed/.malformed")
  require.Error(t, err)
- require.Empty(t, pkg)
+ require.Empty(t, mod)
+ require.Empty(t, client)
+
+ mod, client, err = ExtractModuleName("package.Client")
+ require.NoError(t, err)
+ require.Equal(t, "package", mod)
+ require.Equal(t, "package.Client", client)
+
+ mod, client, err = ExtractModuleName("malformed")
+ require.Error(t, err)
+ require.Empty(t, mod)
+ require.Empty(t, client)
+
+ mod, client, err = ExtractModuleName(".malformed")
+ require.Error(t, err)
+ require.Empty(t, mod)
+ require.Empty(t, client)
+
+ mod, client, err = ExtractModuleName("malformed.")
+ require.Error(t, err)
+ require.Empty(t, mod)
+ require.Empty(t, client)
+
+ mod, client, err = ExtractModuleName("")
+ require.Error(t, err)
+ require.Empty(t, mod)
+ require.Empty(t, client)
 }