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

feat: price-feeder: telemetry additions #673

Merged
merged 7 commits into from
Mar 23, 2022
Merged

Conversation

adamewozniak
Copy link
Collaborator

Description

Adds some measurements for Websocket reconnection attempts & candle / ticker messages being received

closes: #637


Author Checklist

All items are required. Please add a note to the item if the item is not applicable and
please add links to any relevant follow up issues.

I have...

  • included the correct type prefix in the PR title
  • added appropriate labels to the PR
  • added ! to the type prefix if API or client breaking change
  • targeted the correct branch (see PR Targeting)
  • provided a link to the relevant issue or specification
  • added a changelog entry to CHANGELOG.md
  • included comments for documenting Go code
  • updated the relevant documentation or specification
  • reviewed "Files changed" and left comments if necessary
  • confirmed all CI checks have passed

Reviewers Checklist

All items are required. Please add a note if the item is not applicable and please add
your handle next to the items reviewed if you only reviewed selected items.

I have...

  • confirmed the correct type prefix in the PR title
  • confirmed ! in the type prefix if API or client breaking change
  • confirmed all author checklist items have been addressed
  • reviewed state machine logic
  • reviewed API design and naming
  • reviewed documentation is accurate
  • reviewed tests and test coverage
  • manually tested (if applicable)

"type",
"ticker",
"provider",
config.ProviderBinance,
Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Just using the only constants that carry the names of the providers here - maybe we should have an additional set of constants in the providers package, but that seems a bit overkill for this

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Would also like @RafilxTenfen 's input on any other metrics that might be useful

@adamewozniak adamewozniak marked this pull request as ready for review March 23, 2022 00:49
@adamewozniak adamewozniak requested a review from a team as a code owner March 23, 2022 00:49
@RafilxTenfen
Copy link
Contributor

@adamewozniak

The telemetry is going to increase the count when peggo uses the price-feeder?
If so, do you think we should differentiate the usage with an key?

@RafilxTenfen
Copy link
Contributor

RafilxTenfen commented Mar 23, 2022

What do you think about adding telemetry to subscribedPairs? like:

       ....
       	p.setSubscribedPairs(cps...)
	telemetry.IncrCounter(
		float32(len(cps)),
		"websocket",
		"subscribe",
		"currency_pairs",
		"provider",
		config.ProviderBinance,
	)

@adamewozniak
Copy link
Collaborator Author

@adamewozniak

The telemetry is going to increase the count when peggo uses the price-feeder? If so, do you think we should differentiate the usage with an key?

iirc it would only count if telemetry were enabled (e.g. if you called telemetry.New), so I don't really think we need a price.feeder key to differentiate, I don't think peggo has its own telemetry

Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>
Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>
@toteki
Copy link
Member

toteki commented Mar 23, 2022

Looks good (but give it a final check before merging)

@toteki toteki merged commit 0c65084 into main Mar 23, 2022
@toteki toteki deleted the woz/pf/telemetry-additions branch March 23, 2022 19:07
mergify bot pushed a commit that referenced this pull request Mar 23, 2022
* add telemetry for websocket reconnection attempts & receiving candles / ticker messages

* add "missed vote" telemetry

* fix telemetry src

* add a telemetry counter for subscriptions to cps

* Update price-feeder/oracle/provider/gate.go

Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>

* Update price-feeder/oracle/provider/huobi.go

Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>

* Batch of 3

Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>
(cherry picked from commit 0c65084)
adamewozniak added a commit that referenced this pull request Mar 23, 2022
* add telemetry for websocket reconnection attempts & receiving candles / ticker messages

* add "missed vote" telemetry

* fix telemetry src

* add a telemetry counter for subscriptions to cps

* Update price-feeder/oracle/provider/gate.go

Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>

* Update price-feeder/oracle/provider/huobi.go

Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>

* Batch of 3

Co-authored-by: Adam Moser <63419657+toteki@users.noreply.github.com>
(cherry picked from commit 0c65084)

Co-authored-by: Adam Wozniak <29418299+adamewozniak@users.noreply.github.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Evaluate price-feeder telemetry metrics
3 participants