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

Watchers should include it's version in requests towards ChildChain block.get #1710

Open
InoMurko opened this issue Aug 28, 2020 · 7 comments

Comments

@InoMurko
Copy link
Contributor

InoMurko commented Aug 28, 2020

So that we have some information about clients in the network and how our releases are used, we should include the release version (version-SHA) as a HTTP header sent to the childchain.

Specs

Childchain

Feature: Process `X-Watcher-Version` HTTP header
  The `X-Watcher-Version` enables the operator to understand what clients are making the requests
  and is used to develop maintenance strategy e.g. versioning and upgrade path.

Rule: A request is processed if it contains `X-Watcher-Version` HTTP header

Scenario: Alice sends a `X-Watcher-Version` header with a valid watcher version
Scenario: Alice sends a `X-Watcher-Version` header with an arbitrary value
Scenario: Alice sends a request without the `X-Watcher-Version` header

# We cannot enable this rule yet since most running watchers would not have implemented this yet.
@wip
Rule: A request is rejected if it does not contain `X-Watcher-Version` HTTP header

Scenario: Alice sends a `X-Watcher-Version` header with an arbitrary value
Scenario: Alice sends a request without the `X-Watcher-Version` header

Rule: Each `X-Watcher-Version` value is counted and submitted as a metric to Datadog

Scenario: Alice sends a `X-Watcher-Version` header with a valid watcher version
Scenario: Alice sends a `X-Watcher-Version` header with a different watcher version
Scenario: Alice sends a `X-Watcher-Version` header with an arbitrary value
Scenario: Alice sends a request without the `X-Watcher-Version` header

Watcher

Feature: Submit `X-Watcher-Version` HTTP header with all requests to the childchain
  The `X-Watcher-Version` enables the operator to understand what clients are making the requests
  and is used to develop maintenance strategy e.g. versioning and upgrade path.

Rule: Each childchain request sent must contain the `X-Watcher-Version` with the value being the watcher version

Scenario: Alice sends a request to the childchain
Scenario: Alice sends a request to a rootchain node
@InoMurko
Copy link
Contributor Author

The header+value of the HTTP header should be:
X-Watcher-Version: 1.0.3+cb41972

@unnawut
Copy link
Contributor

unnawut commented Sep 11, 2020

@InoMurko @achiurizo I've added this one to this cycle's board to propose including this in the cycle. With hope that you'd accept it to the team's todo list 😂

Also trialed a bit of gherkin specs (see the issue description). The content I wrote doesn't feel too right so do let me know if you have any improvement suggestions.

@achiurizo
Copy link
Contributor

Let's consider using a User-Agent schema instead: https://meet.google.com/linkredirect?authuser=1&dest=https%3A%2F%2Fdeveloper.mozilla.org%2Fen-US%2Fdocs%2FWeb%2FHTTP%2FHeaders%2FUser-Agent

This allows us more flexibility with different watcher implementations.

@pgebal
Copy link
Contributor

pgebal commented Nov 12, 2020

@InoMurko @achiurizo
Why do we have to validate X-Watcher-Version in childchain? Anyone can set that header when making a request. Client talking to childchain does not have to be our watcher anyway.
I would rather add the header (or User-Agent header) in watcher and not do any checks on that in the child-chain.

And what about User-Agent vs X-Watcher-Version? I would prefer User-Agent than our custom header.

@InoMurko
Copy link
Contributor Author

InoMurko commented Nov 12, 2020

@achiurizo added the word "valid" in the specification, but he didn't mean validation on the childchain side. As long as our watchers set the name and version it's fine and if it reaches the childchain, even better.

@pgebal
Copy link
Contributor

pgebal commented Dec 1, 2020

@InoMurko @achiurizo Guys, I can't find any value in collecting metrics for x-watcher-version in childchains. Whatever we count there does not reflect reality, anyone can put an arbitrary value into that header.
Why do we want to look on fake data? I think we'll be good with just logging the header.

@pgebal
Copy link
Contributor

pgebal commented Dec 14, 2020

It's done in watcher. As I understand this task is currently on hold for childchain v2. I unassigned myself from this task.

@pgebal pgebal removed their assignment Dec 14, 2020
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

No branches or pull requests

4 participants