From 8c1a49630f53cbaf27031222364d6d5162dc8c7d Mon Sep 17 00:00:00 2001 From: Joel Hendrix Date: Mon, 6 Nov 2023 10:45:38 -0800 Subject: [PATCH] Rename clientName param to moduleName (#21893) Per spec, the tracer name should be the name of the module. Removed some unnecessary code. --- sdk/azcore/CHANGELOG.md | 1 + sdk/azcore/arm/client.go | 16 +++---- sdk/azcore/arm/client_test.go | 12 ++---- sdk/azcore/core.go | 16 +++---- sdk/azcore/core_test.go | 18 ++------ sdk/azcore/internal/shared/shared.go | 24 ----------- sdk/azcore/internal/shared/shared_test.go | 52 ----------------------- sdk/azcore/tracing/tracing.go | 10 ++--- 8 files changed, 24 insertions(+), 125 deletions(-) diff --git a/sdk/azcore/CHANGELOG.md b/sdk/azcore/CHANGELOG.md index 12edcf3fbd94..d8b31a8d1270 100644 --- a/sdk/azcore/CHANGELOG.md +++ b/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) diff --git a/sdk/azcore/arm/client.go b/sdk/azcore/arm/client.go index aa34575f66fb..c373cc43fd50 100644 --- a/sdk/azcore/arm/client.go +++ b/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 } diff --git a/sdk/azcore/arm/client_test.go b/sdk/azcore/arm/client_test.go index a95d0c44882b..fd7133417896 100644 --- a/sdk/azcore/arm/client_test.go +++ b/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, }, diff --git a/sdk/azcore/core.go b/sdk/azcore/core.go index 9a13ffe90691..8eef8633a7e8 100644 --- a/sdk/azcore/core.go +++ b/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}) } diff --git a/sdk/azcore/core_test.go b/sdk/azcore/core_test.go index 15039153ccb2..a63e2ac8456b 100644 --- a/sdk/azcore/core_test.go +++ b/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) diff --git a/sdk/azcore/internal/shared/shared.go b/sdk/azcore/internal/shared/shared.go index 6d572836baac..16bc105f4811 100644 --- a/sdk/azcore/internal/shared/shared.go +++ b/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 diff --git a/sdk/azcore/internal/shared/shared_test.go b/sdk/azcore/internal/shared/shared_test.go index 99a287e98e54..b17b37ed5c09 100644 --- a/sdk/azcore/internal/shared/shared_test.go +++ b/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" diff --git a/sdk/azcore/tracing/tracing.go b/sdk/azcore/tracing/tracing.go index fbf4be6269a8..1ade7c560ff1 100644 --- a/sdk/azcore/tracing/tracing.go +++ b/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 }