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

Make GooseUser and GooseUserData cloneable #595

Merged

Conversation

S7evinK
Copy link
Contributor

@S7evinK S7evinK commented Jun 12, 2024

As the title describes, this implements Clone and Debug for GooseUser and GooseUserData.

In our use-case we need to pass GooseUser to a SDK which is doing all the network requests.

Signed-off-by: Till Faelligen <2353100+S7evinK@users.noreply.github.com>
@S7evinK S7evinK changed the title S7evink/cloneable goose user Make GooseUser and GooseUserData cloneable Jun 12, 2024
@jeremyandrews
Copy link
Member

Interesting, thanks for the PR. Be sure to run through cargo fmt which enforce our coding standards.

In your use case, does Goose provide you any metrics? When you clone and pass off the GooseUser, is something other than Reqwest making the requests? Why not set up a proxy? I'm trying to better understand your use case, and the implications of this change.

@S7evinK
Copy link
Contributor Author

S7evinK commented Jun 12, 2024

Thanks for the quick reply. :)

Goose provides metrics in our case which we need to sanitize, as having metrics for each "room" (e.g. /_matrix/client/v3/rooms/{roomId}/join) doesn't make sense as we care about the /join endpoint only.
The underlying matrix-rust-sdk uses reqwest as well, but if we pass it the client provided by GooseUser, we're losing the metrics.

Copy link
Member

@jeremyandrews jeremyandrews left a comment

Choose a reason for hiding this comment

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

I don't see any inherent downside to making things cloneable and debug formatted, thank you!

Note that you can name requests to avoid different paths from creating a different metric. For example, see how Goose Eggs does this with static elements:
https://github.com/tag1consulting/goose-eggs/blob/428939d1634acd3224d1f635433eee56ab98e605/src/lib.rs#L1076

@jeremyandrews jeremyandrews merged commit 7c1e584 into tag1consulting:main Jun 13, 2024
2 checks passed
@S7evinK S7evinK deleted the s7evink/cloneable-goose-user branch June 13, 2024 09:34
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.

None yet

2 participants