Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix/module name change #22258

Closed
wants to merge 6 commits into from
Closed

Fix/module name change #22258

Show file tree
Hide file tree
Changes from 5 commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
29be431
upgrade to latest version of azblob with test fixes
tanyasethi-msft Jan 2, 2024
ca546d6
Merge remote-tracking branch 'origin/main'
tanyasethi-msft Jan 10, 2024
2f107ef
[azdatalake] STG 82 Preview
tanyasethi-msft Jan 10, 2024
5195f1b
Merge remote-tracking branch 'origin/main'
tanyasethi-msft Jan 17, 2024
a784784
module name update
tanyasethi-msft Jan 17, 2024
6e012f6
azdatalake recordings
tanyasethi-msft Jan 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
12 changes: 6 additions & 6 deletions sdk/storage/azblob/internal/shared/shared.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,12 +45,12 @@ const (
const crc64Polynomial uint64 = 0x9A6C9329AC4BC9B5

const (
AppendBlobClient = "azblob/appendblob.Client"
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

As of [email protected], the moduleName param to azcore.NewClient() takes the full module name instead of the package.ClientName. So, all of these constants can go away.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

All the three packages are already using [email protected]. What do you mean by full module name?

The context here is that earlier we were passing ModuleName as azblob/service.Client for example to create the azcore client from service client. In the telemetry policy, this is updated to service.Client from this code. This resulted in the user agent string as azsdk-go-service.Client/v1.2.0 (go1.19.3; Windows_NT).

In this PR we are replacing / with . in module name of respective clients which would give the user agent string, azsdk-go-azblob.service/v1.2.0 (go1.19.3; Windows_NT)

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fully qualified module name e.g. github.com/Azure/azure-sdk-for-go/sdk/storage/azblob.

Originally, we wanted package.ClientName for use by the tracing policy. We've retooled how tracing works, so passing package.ClientName is no longer necessary.

Regarding the telemetry policy, we don't want/need the client name in the User-Agent string. See the guidelines for the format.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In current implementation, "azsdk-go-service.Client/v1.2.0 (go1.19.3; Windows_NT)" this user-agent string is not able to identify whether request was generated by "blob", "datalake" or "files" client as all three can have service client. Hence we are removing the "/" to include "azblob" or "azdatalake" in the string as well.
For the other part "service.Client" does not make much sense for telemetry in any way. If we are interested in just knowing which module was used then "azblob.service" itself tell the whole story ".client" is not adding any value there.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

On FE side we are building telemetry to parase the User-Agent string and dump some of the keywords to a dgrep table. Longer the string or more vivid the string, higher resource consumption between FE and XArgus. "azsdk-go-blob" shall be good enough for us to understand what SDK was used. Does Azure-SDK team have any sort of telemetry already built to identify which customers are using what SDK or which clients?

BlobClient = "azblob/blob.Client"
BlockBlobClient = "azblob/blockblob.Client"
ContainerClient = "azblob/container.Client"
PageBlobClient = "azblob/pageblob.Client"
ServiceClient = "azblob/service.Client"
AppendBlobClient = "azblob.appendblob"
BlobClient = "azblob.blob"
BlockBlobClient = "azblob.blockblob"
ContainerClient = "azblob.container"
PageBlobClient = "azblob.pageblob"
ServiceClient = "azblob.service"
)

var CRC64Table = crc64.MakeTable(crc64Polynomial)
Expand Down
8 changes: 4 additions & 4 deletions sdk/storage/azdatalake/internal/shared/shared.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -25,10 +25,10 @@ const (
)

const (
ServiceClient = "azdatalake/service.Client"
FileSystemClient = "azdatalake/filesystem.Client"
DirectoryClient = "azdatalake/directory.Client"
FileClient = "azdatalake/file.Client"
ServiceClient = "azdatalake.service"
FileSystemClient = "azdatalake.filesystem"
DirectoryClient = "azdatalake.directory"
FileClient = "azdatalake.file"
)

const (
Expand Down
8 changes: 4 additions & 4 deletions sdk/storage/azfile/internal/shared/shared.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,10 +63,10 @@ const (
)

const (
ServiceClient = "azfile/service.Client"
ShareClient = "azfile/share.Client"
DirectoryClient = "azfile/directory.Client"
FileClient = "azfile/file.Client"
ServiceClient = "azfile.service"
ShareClient = "azfile.share"
DirectoryClient = "azfile.directory"
FileClient = "azfile.file"
)

func GetClientOptions[T any](o *T) *T {
Expand Down
Loading