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

fix: configure Lassie user-agent #78

Merged
merged 4 commits into from
Dec 12, 2023
Merged

fix: configure Lassie user-agent #78

merged 4 commits into from
Dec 12, 2023

Conversation

bajtos
Copy link
Member

@bajtos bajtos commented Dec 11, 2023

Fix the user agent used by Rusty Lassie to report the version of the underlying Go Lassie library.

While we want to make SPARK retrievals as much indistinguishable from Saturn retrievals as possible to prevent SPs from prioritising serving SPARK over serving Saturn, we also need the ability to troubleshoot increased load on SPs in the short term.

Based on the discussion below, I decided to use a custom build version -rs in the user agent to distinguish Spark traffic from Saturn traffic.

- user-agent: lassie/v0.0.0-unknown
+ user-agent: lassie/v0.21.0-rs

- Add a build step to ensure the Go Lassie library has the version
  for which we know the commit hash.

- In the build script, set an env var with the Lassie version string
  including the commit hash, e.g. `0.21.0-2cf1121`.

- Add internal configuration field to pass this commit hash from
  the Rust side down to the Go land.

- Add a deamon initialization step to configure Lassie's user agent.

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
@bajtos bajtos requested a review from juliangruber December 11, 2023 15:45
@bajtos
Copy link
Member Author

bajtos commented Dec 11, 2023

@rvagg do you have any concerns about us taking this approach?

@hannahhoward
Copy link

@bajtos we should probably hold off till we validate the issue with traffic on SPs

@gruns
Copy link

gruns commented Dec 11, 2023

We want to make SPARK retrievals as much indistinguishable from Saturn retrievals as possible, otherwise SPs can prioritise serving SPARK over serving Saturn.

hrm. @bajtos this is counter to the point of user agents, though. eg the user agent field becomes useless if every browser announced itself as chrome

if SPs want to block, prioritize, or otherwise manipulate traffic based on user agent, that's their prerogative. put another way, if i was an SP i'd be upset if a bunch of tools from PL all mendaciously used the same user agent so i couldnt discern different bitswap requests

all that said, zooming out, i understand the issue is a bit funky because right now SPs have zero incentive to serve requests. they're not remunerated, just fulfilling the FIL+ contract

@rvagg
Copy link

rvagg commented Dec 12, 2023

@rvagg do you have any concerns about us taking this approach?

Aside from the other points made here about UAs and differentiation, no. It would be good at least to have a UA that matches the actual Lassie version because it does matter and is helpful for debugging. You are a bit ahead of Saturn atm if you cared about being in sync now https://github.com/filecoin-saturn/L1-node/blob/a3748041f011ba6a85d8c7173dc785c12ac32ffd/Dockerfile#L14

As an aside, my current, evolved, perspective on other concerns about UAs is that we should give as much agency to SPs as possible. It's been one of the biggest problems they've faced - we deliver the software and they wear the costs of it with very little ability to tune those costs. The less they feel in control the more cautious and conservative they'll be and they'll also be less inclined to trust "the network". Similar reasons why we should probably just be telling them to turn everything off but http and use their nginx-fu to take full control of retrievals, throttling and all, to match their level of comfort and their stack's ability to serve retrievals. Differentiating based on UA is a tool in the toolbox, if they don't care about Station validating their junk then let them block. Likewise for Saturn, who should probably be setting up deals to pay SPs for retrievals and everyone who doesn't get remunerated can block if it's a burden.

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
@bajtos
Copy link
Member Author

bajtos commented Dec 12, 2023

Based on the comments above, I reworked my PR to report the same lassie version but using -rs as the build number suffix.

lassie/v0.21.0-rs

This allows SPs to distinguish Stations vs Saturn traffic, which we don't want in the long term. But given the current situation, I think it's useful to keep this distinction for now to simplify troubleshooting.

Thoughts?

Signed-off-by: Miroslav Bajtoš <oss@bajtos.net>
@rvagg
Copy link

rvagg commented Dec 12, 2023

sgtm

@bajtos bajtos enabled auto-merge (squash) December 12, 2023 12:02
@bajtos bajtos merged commit 011814e into main Dec 12, 2023
10 checks passed
@bajtos bajtos deleted the fix-lassie-useragent branch December 12, 2023 12:10
@bajtos
Copy link
Member Author

bajtos commented Dec 12, 2023

Landed, released, and currently on the way to Zinnia: filecoin-station/zinnia#433

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.

6 participants