Skip to content
This repository has been archived by the owner on Aug 2, 2024. It is now read-only.

feat(logs): Display Madara logs instead of Substrate logs #1609

Closed
wants to merge 7 commits into from
Closed

feat(logs): Display Madara logs instead of Substrate logs #1609

wants to merge 7 commits into from

Conversation

azurwastaken
Copy link
Contributor

Pull Request type

Please add the labels corresponding to the type of changes your PR introduces:

  • Feature

What is the current behavior?

Currently madara print substrate logs

Resolves: #1484

What is the new behavior?

  • Added the same logs as substrate but with Starknet related stuff

Does this introduce a breaking change?

No

Other information

I only added Starknet related logs because I don't know what to do with substrate logs, should I remove them directly from the massa fork ?

@tdelabro
Copy link
Collaborator

I don't know what was the goal here, but none of this makes sense.
Those are the wrong logs, to the wrong place.
This service just dump some useful values into our backend. It has nothing to do with block production or consensus. I don't know why you put those logs threre

@tdelabro tdelabro closed this May 30, 2024
@azurwastaken
Copy link
Contributor Author

I don't know what was the goal here, but none of this makes sense. Those are the wrong logs, to the wrong place. This service just dump some useful values into our backend. It has nothing to do with block production or consensus. I don't know why you put those logs threre

Following #1484 , the goal is to provide starknet/madara related data instead of substrate ones because the substrate datas but maybe there is a better place to put them in madara. In fact i just did as discussed in Telegram.

Could you please show me where you think it should be ? Because, as mentioned in telegram, its an absolute pain to integrate it in the polkadot sdk fork and it kill the perf to retrieve starknet data within it.

@tdelabro
Copy link
Collaborator

its an absolute pain to integrate it in the polkadot sdk fork

If you want log about the consensus, you should put them in the part of the code that handles the consensus, and that is the polkadot SDK

I don't see a good way to work around the fact that the logs should be where the things are happening.

@tdelabro
Copy link
Collaborator

it kill the perf to retrieve starknet data within it.

Logs are informative stuff, you consult once in a while. Perf should not be at stake when we talk about reading logs.
What are you trying to achieve with those logs exactly?

@github-actions github-actions bot locked and limited conversation to collaborators Jun 3, 2024
@azurwastaken azurwastaken deleted the fix_logs branch June 6, 2024 16:21
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

feat(logs): Display Madara logs instead of Substrate logs
3 participants