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

Jump to bottom

[iota-analytics-indexer] Add README.md #4470

Merged
merged 6 commits into from
Dec 20, 2024
Merged

[iota-analytics-indexer] Add README.md #4470

merged 6 commits into from
Dec 20, 2024

Conversation

tomxey
Copy link
Contributor

@tomxey tomxey commented Dec 12, 2024 •
edited
Loading

Description of change

Add README.md for iota-analytics-indexer

Links to any relevant issues

#3976

Type of change

Choose a type of change, and delete any options that are not relevant.

  • Documentation Fix

How the change has been tested

N.A.

Change checklist

Tick the boxes that are relevant to your changes, and delete any items that are not.

  • I have followed the contribution guidelines for this project
  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation

@iota-ci iota-ci added infrastructure Issues related to the Infrastructure Team sc-platform Issues related to the Smart Contract Platform group. labels Dec 12, 2024
@tomxey tomxey self-assigned this Dec 12, 2024
@tomxey tomxey force-pushed the sc-platform/review-iota-analytics-indexer branch 7 times, most recently from c6b09cf to 687df7d Compare December 12, 2024 12:08
@tomxey tomxey force-pushed the sc-platform/review-iota-analytics-indexer branch from 687df7d to 9e109b0 Compare December 12, 2024 12:19
@tomxey tomxey marked this pull request as ready for review December 12, 2024 12:20
@tomxey tomxey requested a review from a team as a code owner December 12, 2024 12:20
@kodemartin
Copy link
Contributor

Had a first quick read:

  • I don't get what metrics are collected though? How does it compare to the indexer-analytical-worker?
  • Could you elaborate on the BigQuery/Snowflake schemas used?
  • What is BigQuery and Snowflake in the first place? It would be helpful to write a few things and provide references links for digging deeper.

@lzpap lzpap linked an issue Dec 12, 2024 that may be closed by this pull request
[iota-analytics-indexer]: Review application use cases and collected metrics #3976
Closed
@tomxey
Fixing review remarks:
193811b 
 - Added more info about SnowFlake and BigQuery
 - Expanded a bit the comparison with iota-indexer
 - Added more info about schemas for exported data
@tomxey
Copy link
Contributor Author

tomxey commented Dec 17, 2024

Had a first quick read:

* I don't get what metrics are collected though? How does it compare to the `indexer-analytical-worker`?

* Could you elaborate on the `BigQuery/Snowflake` schemas used?

* What is `BigQuery` and `Snowflake` in the first place? It would be helpful to write a few things and provide references links for digging deeper.

@kodemartin Pushed changes that are addressing those remarks.

kodemartin
kodemartin approved these changes Dec 18, 2024
View reviewed changes
Copy link
Contributor

@kodemartin kodemartin left a comment

Choose a reason for hiding this comment

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

Looks good @tomxey ! Left a few optional suggestions.

Also note that the diagram does not look very clear on dark mode (there are some hatch lines overlapping with text for example).

crates/iota-analytics-indexer/README.md Outdated

Those metrics are computed by a separate analytical worker instance of the indexer, but it uses the main DB as the main indexer instance.

It seems that some of the values stored in main indexer tables by iota-indexer's `fullnode_sync_worker` are only stored there for analytical purposes (move calls, tx recipients) and could potentially be not processed/stored if not for analytical reasons.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Phrasing

Suggested change
It seems that some of the values stored in main indexer tables by iota-indexer's `fullnode_sync_worker` are only stored there for analytical purposes (move calls, tx recipients) and could potentially be not processed/stored if not for analytical reasons.
It seems that some of the values stored in main indexer tables by iota-indexer's `fullnode_sync_worker` are only stored there for analytical purposes (move calls, tx recipients) and could potentially be excluded from further processing if it were not for analytical reasons.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

crates/iota-analytics-indexer/README.md Outdated

The `iota-analytics-indexer` is not computing any analytical metrics directly.
It is only exporting data for further processing via external tools (BigQuery/SnowFlake).
Functionality from `iota-indexer` that is not required to serve user JSON RPC/GraphQL requests could potentially be moved away from `iota-indexer` and served by some other tool based on data exported by the `iota-analytics-indexer`.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: Phrasing (if I get it right)

Suggested change
Functionality from `iota-indexer` that is not required to serve user JSON RPC/GraphQL requests could potentially be moved away from `iota-indexer` and served by some other tool based on data exported by the `iota-analytics-indexer`.
On this premise, the functionality in `iota-indexer` that is currently used for extracting analytics (and thus unrelated to the JSON-RPC/GraphQL service) could be moved out and delegated to another tool that processes data exported by `iota-analytics-indexer`.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

crates/iota-analytics-indexer/README.md Outdated
The `iota-analytics-indexer` is not computing any analytical metrics directly.
It is only exporting data for further processing via external tools (BigQuery/SnowFlake).
Functionality from `iota-indexer` that is not required to serve user JSON RPC/GraphQL requests could potentially be moved away from `iota-indexer` and served by some other tool based on data exported by the `iota-analytics-indexer`.
The sync logic in `iota-indexer` could be simplified a bit then to store only data that is needed to serve user requests.
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit:

Suggested change
The sync logic in `iota-indexer` could be simplified a bit then to store only data that is needed to serve user requests.
Then the sync logic in `iota-indexer` could be simplified as well to store only data that is needed for the purposes of the RPC APIs.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

crates/iota-analytics-indexer/README.md Outdated
- TRANSACTION


> Note: The following rust structs currently do not have DB schemas prepared:
Copy link
Contributor

Choose a reason for hiding this comment

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

Nit: For further emphasis (see also GH alerts)

Suggested change
> Note: The following rust structs currently do not have DB schemas prepared:
> [!NOTE]
> The following rust structs currently do not have DB schemas prepared:

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks for the tip. Done.

sergiupopescu199
sergiupopescu199 approved these changes Dec 20, 2024
View reviewed changes
Copy link
Contributor

@sergiupopescu199 sergiupopescu199 left a comment

Choose a reason for hiding this comment

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

LGTM ✨

@tomxey
Copy link
Contributor Author

tomxey commented Dec 20, 2024

Also note that the diagram does not look very clear on dark mode (there are some hatch lines overlapping with text for example).

@kodemartin Changed styling of the diagram. Now it should be readable both in dark and light modes.

@tomxey tomxey merged commit 1661d22 into develop Dec 20, 2024
42 checks passed
@tomxey tomxey deleted the sc-platform/review-iota-analytics-indexer branch December 20, 2024 15:13
miker83z pushed a commit that referenced this pull request Dec 20, 2024
@tomxey @miker83z
[iota-analytics-indexer] Add README.md (#4470)
4a55d6f 
* Add README.md

* Fixing review remarks:
 - Added more info about SnowFlake and BigQuery
 - Expanded a bit the comparison with iota-indexer
 - Added more info about schemas for exported data

* Fix review remarks about README contents

* Applied automatic dprint format

* Use standard mermaid diagram theme

* Style diagram to look better in both dark and light modes
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@sergiupopescu199 sergiupopescu199 sergiupopescu199 approved these changes

@kodemartin kodemartin kodemartin approved these changes

@samuel-rufi samuel-rufi Awaiting requested review from samuel-rufi

Assignees

@tomxey tomxey

Labels
infrastructure Issues related to the Infrastructure Team sc-platform Issues related to the Smart Contract Platform group.
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

[iota-analytics-indexer]: Review application use cases and collected metrics
4 participants
@tomxey @kodemartin @sergiupopescu199 @iota-ci