Rename clientName param to moduleName (#21893)

Per spec, the tracer name should be the name of the module.
Removed some unnecessary code.

Author: jhendrixMSFT

sdk/azcore/CHANGELOG.md

@@ -23,6 +23,7 @@
 ### Other Changes
 
 * Skip generating trace info for no-op tracers.
+* The `clientName` paramater in client constructors has been renamed to `moduleName`.
 
 ## 1.9.0-beta.1 (2023-10-05)
 

sdk/azcore/arm/client.go

@@ -28,17 +28,11 @@ 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 ("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
+//   - moduleName - the fully qualified name of the module where the client is defined; used by the telemetry policy and tracing provider.
+//   - moduleVersion - the semantic version of the module; used by the telemetry policy and tracing provider.
 //   - 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) {
-	mod, client, err := shared.ExtractModuleName(clientName)
-	if err != nil {
-		return nil, err
-	}
-
+func NewClient(moduleName, moduleVersion string, cred azcore.TokenCredential, options *ClientOptions) (*Client, error) {
 	if options == nil {
 		options = &ClientOptions{}
 	}
@@ -53,12 +47,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(mod, moduleVersion, cred, runtime.PipelineOptions{}, options)
+	pl, err := armruntime.NewPipeline(moduleName, moduleVersion, cred, runtime.PipelineOptions{}, options)
 	if err != nil {
 		return nil, err
 	}
 
-	tr := options.TracingProvider.NewTracer(client, moduleVersion)
+	tr := options.TracingProvider.NewTracer(moduleName, moduleVersion)
 	return &Client{ep: ep, pl: pl, tr: tr}, nil
 }
 

sdk/azcore/arm/client_test.go

@@ -24,14 +24,14 @@ func (mc fakeCredential) GetToken(ctx context.Context, options policy.TokenReque
 }
 
 func TestNewClient(t *testing.T) {
-	client, err := NewClient("package.Client", "v1.0.0", fakeCredential{}, nil)
+	client, err := NewClient("module", "v1.0.0", fakeCredential{}, nil)
 	require.NoError(t, err)
 	require.NotNil(t, client)
 	require.Equal(t, cloud.AzurePublic.Services[cloud.ResourceManager].Endpoint, client.Endpoint())
 	require.NotZero(t, client.Pipeline())
 	require.Zero(t, client.Tracer())
 
-	client, err = NewClient("package.Client", "", fakeCredential{}, &ClientOptions{
+	client, err = NewClient("module", "", fakeCredential{}, &ClientOptions{
 		ClientOptions: azcore.ClientOptions{
 			Cloud: cloud.AzureChina,
 			Telemetry: policy.TelemetryOptions{
@@ -45,11 +45,7 @@ func TestNewClient(t *testing.T) {
 }
 
 func TestNewClientError(t *testing.T) {
-	client, err := NewClient("malformed", "v1.0.0", fakeCredential{}, nil)
-	require.Error(t, err)
-	require.Nil(t, client)
-
-	client, err = NewClient("package.Client", "malformed", fakeCredential{}, nil)
+	client, err := NewClient("module", "malformed", fakeCredential{}, nil)
 	require.Error(t, err)
 	require.Nil(t, client)
 
@@ -60,7 +56,7 @@ func TestNewClientError(t *testing.T) {
 			},
 		},
 	}
-	client, err = NewClient("package.Client", "v1.0.0", fakeCredential{}, &ClientOptions{
+	client, err = NewClient("module", "v1.0.0", fakeCredential{}, &ClientOptions{
 		ClientOptions: azcore.ClientOptions{
 			Cloud: badCloud,
 		},

sdk/azcore/core.go

@@ -101,17 +101,11 @@ type Client struct {
 }
 
 // NewClient creates a new Client instance with the provided values.
-//   - 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
+//   - moduleName - the fully qualified name of the module where the client is defined; used by the telemetry policy and tracing provider.
+//   - moduleVersion - the semantic version of the module; used by the telemetry policy and tracing provider.
 //   - 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) {
-	mod, client, err := shared.ExtractModuleName(clientName)
-	if err != nil {
-		return nil, err
-	}
-
+func NewClient(moduleName, moduleVersion string, plOpts runtime.PipelineOptions, options *ClientOptions) (*Client, error) {
 	if options == nil {
 		options = &ClientOptions{}
 	}
@@ -122,9 +116,9 @@ func NewClient(clientName, moduleVersion string, plOpts runtime.PipelineOptions,
 		}
 	}
 
-	pl := runtime.NewPipeline(mod, moduleVersion, plOpts, options)
+	pl := runtime.NewPipeline(moduleName, moduleVersion, plOpts, options)
 
-	tr := options.TracingProvider.NewTracer(client, moduleVersion)
+	tr := options.TracingProvider.NewTracer(moduleName, moduleVersion)
 	if tr.Enabled() && plOpts.Tracing.Namespace != "" {
 		tr.SetAttributes(tracing.Attribute{Key: shared.TracingNamespaceAttrName, Value: plOpts.Tracing.Namespace})
 	}

sdk/azcore/core_test.go

@@ -128,16 +128,6 @@ func TestNewClient(t *testing.T) {
 	require.NotNil(t, client)
 }
 
-func TestNewClientError(t *testing.T) {
-	client, err := NewClient("malformed", "v1.0.0", runtime.PipelineOptions{}, nil)
-	require.Error(t, err)
-	require.Nil(t, client)
-
-	client, err = NewClient("package.Client", "malformed", runtime.PipelineOptions{}, nil)
-	require.Error(t, err)
-	require.Nil(t, client)
-}
-
 func TestNewClientTracingEnabled(t *testing.T) {
 	srv, close := mock.NewServer()
 	defer close()
@@ -184,7 +174,7 @@ func TestClientWithClientName(t *testing.T) {
 	var clientName string
 	var modVersion string
 	var attrString string
-	client, err := NewClient("module/package.Client", "v1.0.0", runtime.PipelineOptions{
+	client, err := NewClient("module", "v1.0.0", runtime.PipelineOptions{
 		Tracing: runtime.TracingOptions{
 			Namespace: "Widget.Factory",
 		},
@@ -210,7 +200,7 @@ func TestClientWithClientName(t *testing.T) {
 	require.NotNil(t, client)
 	require.NotZero(t, client.Pipeline())
 	require.NotZero(t, client.Tracer())
-	require.EqualValues(t, "package.Client", clientName)
+	require.EqualValues(t, "module", clientName)
 	require.EqualValues(t, "v1.0.0", modVersion)
 
 	const requestEndpoint = "/subscriptions/00000000-0000-0000-0000-000000000000/resourceGroups/fakeResourceGroupo/providers/Microsoft.Storage/storageAccounts/fakeAccountName"
@@ -221,8 +211,8 @@ func TestClientWithClientName(t *testing.T) {
 	require.NoError(t, err)
 	require.EqualValues(t, "az.namespace:Widget.Factory", attrString)
 
-	newClient := client.WithClientName("other.Client")
-	require.EqualValues(t, "other.Client", clientName)
+	newClient := client.WithClientName("other")
+	require.EqualValues(t, "other", clientName)
 	require.EqualValues(t, "v1.0.0", modVersion)
 	require.EqualValues(t, client.Pipeline(), newClient.Pipeline())
 	_, err = newClient.Pipeline().Do(req)

sdk/azcore/internal/shared/shared.go

@@ -87,30 +87,6 @@ func ValidateModVer(moduleVersion string) error {
 	return nil
 }
 
-// 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 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
-	}
-
-	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
-}
-
 // ContextWithDeniedValues wraps an existing [context.Context], denying access to certain context values.
 // Pipeline policies that create new requests to be sent down their own pipeline MUST wrap the caller's
 // context with an instance of this type. This is to prevent context values from flowing across disjoint

sdk/azcore/internal/shared/shared_test.go

@@ -85,58 +85,6 @@ func TestValidateModVer(t *testing.T) {
 	require.Error(t, ValidateModVer("v1.2"))
 }
 
-func TestExtractModuleName(t *testing.T) {
-	mod, client, err := ExtractModuleName("module/package.Client")
-	require.NoError(t, err)
-	require.Equal(t, "module", 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/malformed")
-	require.Error(t, err)
-	require.Empty(t, mod)
-	require.Empty(t, client)
-
-	mod, client, err = ExtractModuleName("malformed/malformed.")
-	require.Error(t, err)
-	require.Empty(t, mod)
-	require.Empty(t, client)
-
-	mod, client, err = ExtractModuleName("malformed/.malformed")
-	require.Error(t, err)
-	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)
-}
-
 func TestContextWithDeniedValues(t *testing.T) {
 	type testKey struct{}
 	const value = "value"

sdk/azcore/tracing/tracing.go

@@ -31,12 +31,12 @@ type Provider struct {
 	newTracerFn func(name, version string) Tracer
 }
 
-// NewTracer creates a new Tracer for the specified name and version.
-//   - name - the name of the tracer object, typically the fully qualified name of the service client
-//   - version - the version of the module in which the service client resides
-func (p Provider) NewTracer(name, version string) (tracer Tracer) {
+// NewTracer creates a new Tracer for the specified module name and version.
+//   - module - the fully qualified name of the module
+//   - version - the version of the module
+func (p Provider) NewTracer(module, version string) (tracer Tracer) {
 	if p.newTracerFn != nil {
-		tracer = p.newTracerFn(name, version)
+		tracer = p.newTracerFn(module, version)
 	}
 	return
 }