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

Add daemon to the user agent when the CLI is started in daemon mode #2789

Open
wants to merge 2 commits into
base: master
Choose a base branch
from
Open

Add daemon to the user agent when the CLI is started in daemon mode #2789

Changes from 1 commit
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
bdfdff6
Add `daemon` to the user agent when the CLI is started in daemon mode
MatteoPologruto Dec 18, 2024
1dc53f7
Use `feedback.Fatal` to be consistent with the rest of the codebase
MatteoPologruto Dec 18, 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
11 changes: 11 additions & 0 deletions internal/cli/daemon/daemon.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,17 @@ func NewCommand(srv rpc.ArduinoCoreServiceServer, settings *rpc.Configuration) *
if maxGRPCRecvMsgSize < 1024 {
feedback.Fatal(i18n.Tr("%s must be >= 1024", "--max-grpc-recv-message-size"), feedback.ErrBadArgument)
}

// The user agent should include "daemon" for analytics purposes
_, err := srv.SettingsSetValue(cmd.Context(), &rpc.SettingsSetValueRequest{
Comment on lines +70 to +71
Copy link
Member

Choose a reason for hiding this comment

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

The user agent should include "daemon" for analytics purposes

Actually no, it should infer the user agent from the gRPC client:

arduino-cli/commands/instances.go

Lines 67 to 70 in 6d9c930

var userAgent string
if md, ok := metadata.FromIncomingContext(ctx); ok {
userAgent = strings.Join(md.Get("user-agent"), " ")
}

I think the Arduino IDE is passing something like arduino-ide/x.y.z into this string.
We should understand why this string is not propagated.

Key: "network.user_agent_ext",
ValueFormat: "cli",
EncodedValue: "daemon",
})
Comment on lines +71 to +75
Copy link
Contributor

Choose a reason for hiding this comment

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

What should we do in case this value is not empty?

  1. Should we always override with the daemon value
  2. should we append to the previous value
  3. should we honor the user setting?

/cc @cmaglie

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 we should go with the third option and honor the user setting. If it's okay with you, I'm going to make this change.

Copy link
Contributor

Choose a reason for hiding this comment

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

@MatteoPologruto I agree. Let's ping also @cmaglie and @Xayton as I don't know if they might have better insight

Copy link
Member

Choose a reason for hiding this comment

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

Agreed with option 3.

if err != nil {
// Should never happen...
panic("Failed to set default value for network.user_agent_ext: " + err.Error())
MatteoPologruto marked this conversation as resolved.
Show resolved Hide resolved
}
},
Run: func(cmd *cobra.Command, args []string) {
runDaemonCommand(srv, daemonPort, debugFile, debug, daemonize, debugFiltersArg, maxGRPCRecvMsgSize)
Expand Down
Loading