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.

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

SDK-3949: Introduce sending client information with requests #164

Merged
merged 2 commits into from
Feb 13, 2023

Conversation

ewanharris
Copy link
Contributor

🔧 Changes

Introduces telemetry to the package which is being sent in the (new) Auth0-Client header.

By default the telemetry data sent is for the go-auth0 package however, it can be overridden by passing a custom Telemetry to the WithTelemetry function on API.New. The structure is as follows:

  • name: A string that is name of the "thing" the telemetry data is for
  • version: A string that is version of the "thing" the telemetry data is for
  • env: A object of strings that is a grab-bag for any other data that the consumer of go-auth0 wishes to send, like OS version for example

By calling WithNoTelemetry the Auth0-Client header is not sent at all

📚 References

🔬 Testing

I'm working on figuring out how to test this to make sure the data shows up as expected (rather than works in theory)

📝 Checklist

  • All new/changed/fixed functionality is covered by tests (or N/A)
  • I have added documentation for all new/changed functionality (or N/A)

@ewanharris ewanharris requested a review from a team as a code owner February 9, 2023 18:00
@codecov-commenter
Copy link

codecov-commenter commented Feb 9, 2023

Codecov Report

Base: 94.95% // Head: 94.75% // Decreases project coverage by -0.20% ⚠️

Coverage data is based on head (c336a83) compared to base (7eef571).
Patch coverage: 67.39% of modified lines in pull request are covered.

Additional details and impacted files
@@            Coverage Diff             @@
##             main     #164      +/-   ##
==========================================
- Coverage   94.95%   94.75%   -0.20%     
==========================================
  Files          38       38              
  Lines        6538     6578      +40     
==========================================
+ Hits         6208     6233      +25     
- Misses        266      276      +10     
- Partials       64       69       +5     
Impacted Files Coverage Δ
internal/client/client.go 65.00% <48.27%> (-5.33%) ⬇️
management/management.go 100.00% <100.00%> (ø)
management/management_option.go 45.83% <100.00%> (+12.50%) ⬆️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

"github.com/auth0/go-auth0"
)

// UserAgent is the default user agent string.
var UserAgent = fmt.Sprintf("Go-Auth0-SDK/%s", auth0.Version)

// Telemetry is the structure used to send telemetry data in the "Auth0-Client" header.
type Telemetry struct {
Copy link
Contributor

Choose a reason for hiding this comment

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

While some of our older SDKs still call this Telemetry, more recently we've started to call it UserAgent instead, as "telemetry" can often have negative connotations and raise eyebrows.
For example: auth0/Auth0.Android#398

Please consider renaming the feature.

Copy link
Contributor

Choose a reason for hiding this comment

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

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think that's a fair point, unfortunately this package already exposes a configurable user agent so I'm not sure I can rename it to that, in SPA-JS it's called Auth0Client, what do you think about that?

Copy link
Contributor

@Widcket Widcket Feb 10, 2023

Choose a reason for hiding this comment

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

Is that user agent string public? If not, you can change that as well. Otherwise, the SPA SDK solution is fine 👍🏼

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Configuring the user agent is public, by calling WithUserAgent on a management.New call and is used by auth0 cli and terraform currently

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Pushed c336a83 to change this, I ending up using Auth0ClientInfo to try and convey that this is information about the client rather than a client that can be used to call API methods (if that makes sense)

internal/client/client.go Outdated Show resolved Hide resolved
@ewanharris
Copy link
Contributor Author

ewanharris commented Feb 10, 2023

CI is failing currently because of changes in golangci-lint as we always pull the latest. Will investigate that on Monday but solutions are either pin to a specific golangci-lint version or update to use go 1.19 (at a minimum)

…ent` header

By default the telemetry data sent is for the go-auth0 package however, it can be overridden by
passing a custom Telemetry to the `WithTelemetry` function on `API.New`. The structure is as follows:
    - name: A string that is name of the "thing" the telemetry data is for
    - version: A string that is version of the "thing" the telemetry data is for
    - env: A object of strings that is a grab-bag for any other data that the consumer of go-auth0 wishes to send, like OS version for example

By calling `WithNoTelemetry` the `Auth0-Client` header is not sent at all
@ewanharris ewanharris changed the title [SDK-3949] Introduce telemetry into the package SDK-3949: Introduce sending client information with requests Feb 13, 2023
@ewanharris ewanharris merged commit aa9273b into main Feb 13, 2023
@ewanharris ewanharris deleted the feat/SDK-3949-telemetry branch February 13, 2023 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants