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

Debuglog as events #1611

Closed
wants to merge 2 commits into from
Closed

Conversation

FlyveHest
Copy link

@FlyveHest FlyveHest commented Feb 21, 2021

Changed debuglog in sync module to be user configurable via debugLogType option given to createClient call.

Can be one of

  • 'stdout' : Default if not specified, behaves as module has always done, outputting debug logging to console
  • 'event' : Emits a debuglog event with string to be logged as data, so the client can incorporate debug logging into their own logging framework, if they so wish
  • 'off' : Turns off debugloggin in sync module

Signed-off-by: Peter Reinhold peter_github@reinhold.dk


This change is marked as an internal change (Task), so will not be included in the changelog.

Peter Reinhold (FlyveHest) added 2 commits February 21, 2021 13:22
… stdout (default and same behaviour as before, emit a debuglog event or be turned off)
@SimonBrandner
Copy link
Contributor

@FlyveHest, please include a sign-off as described here

@FlyveHest
Copy link
Author

@FlyveHest, please include a sign-off as described here

Is editing the PR text sufficient?

@SimonBrandner
Copy link
Contributor

@FlyveHest, yes! 👍

Copy link
Collaborator

@jryans jryans left a comment

Choose a reason for hiding this comment

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

Hmm, why only in the sync module and not the entire JS SDK...?

I'd also like to see an approach that doesn't require adding several arguments to every logging call if possible.

@FlyveHest
Copy link
Author

FlyveHest commented Feb 27, 2021

Mostly because with my, at the time, more limited exposure to the SDK, hadn't seen the output issue elsewhere.

It seems like debug logging has been up to the individual part developers, as there are multiple ways of doing it, depending on where in the project you are.

I recently got encryption working, and got a ton of debug noise there as well, and the way its implemented there is different than in the sync module.

But, i'll try and take a look at a broader scope when I have some more time.

If this PR is rejected, it would be really nice if DEBUG could be set to false in the committed code, at least. (Personal opinion)

@MadLittleMods
Copy link
Contributor

Slightly related to element-hq/element-web#21532

@MadLittleMods MadLittleMods added Z-Community-PR Issue is solved by a community member's PR T-Task Tasks for the team like planning labels Jun 2, 2022
@richvdh
Copy link
Member

richvdh commented Oct 2, 2023

It looks to me like this PR isn't really going anywhere, so I'm going to go ahead and close it, though I certainly agree that better control over the logging is required. #1970 is maybe a better tracker for this.

@richvdh richvdh closed this Oct 2, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
T-Task Tasks for the team like planning Z-Community-PR Issue is solved by a community member's PR
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants