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

Define log domain #81

Merged
merged 5 commits into from
May 8, 2021
Merged

Define log domain #81

merged 5 commits into from
May 8, 2021

Conversation

peteruithoven
Copy link
Collaborator

@peteruithoven peteruithoven commented Feb 28, 2019

This enables filtering log messages using G_MESSAGES_DEBUG, to do do something like the following to only get the logs from a specific indicator and the wingpanel.

G_MESSAGES_DEBUG=wingpanel-indicator-bluetooth wingpanel

More info: https://developer.gnome.org/glib/stable/glib-Message-Logging.html#log-domains

Using add_project_arguments instead of add_global_arguments seems to make more sense for log domains, making sure logs from subprojects aren't included.

This is a bit of a test. If we like this I'd like to add these log domains to all indicators and plugs. Maybe even Gala plugins, Applications menu plugins.

@tintou
Copy link
Member

tintou commented Mar 6, 2019

I was dreaming of this, it would highly decrease the debugging time. The thing preventing me from doing that is that we have to do it in every single project. I'm wondering if the debug domain is the right one, we are using RDNN everywhere so I wonder why not here 🤷‍♂️

@peteruithoven
Copy link
Collaborator Author

peteruithoven commented Mar 6, 2019

One argument is that these names are included in the logs themselves. Having them long makes for harder to read logs. It requires wider windows to read without word wrapping.

Example of logs when I start wingpanel with G_MESSAGES_DEBUG=all:

** (wingpanel:20895): DEBUG: 12:59:27.017: AbstractWifiInterface.vala:280: New network state: NETWORK_STATE_CONNECTED_WIFI_OK
** (wingpanel:20895): DEBUG: 12:59:27.017: AbstractWifiInterface.vala:159: Update active AP
** (wingpanel:20895): DEBUG: 12:59:27.017: AbstractWifiInterface.vala:175: Active ap: Villa Ramsesdreef
** (wingpanel:20895): DEBUG: 12:59:27.020: VpnInterface.vala:25: Starting VPN Interface
(wingpanel:20895): wingpanel-DEBUG: 12:59:27.021: IndicatorManager.vala:319: network registered
** (wingpanel:20895): DEBUG: 12:59:27.051: Indicator.vala:170: Activating DateTime Indicator
(wingpanel:20895): wingpanel-DEBUG: 12:59:27.051: IndicatorManager.vala:319: datetime registered
(wingpanel:20895): wingpanel-indicator-bluetooth-DEBUG: 12:59:27.133: Manager.vala:63: add_path
(wingpanel:20895): wingpanel-indicator-bluetooth-DEBUG: 12:59:27.133: Manager.vala:63: add_path
(wingpanel:20895): wingpanel-indicator-bluetooth-DEBUG: 12:59:27.136: Manager.vala:63: add_path
(wingpanel:20895): wingpanel-indicator-bluetooth-DEBUG: 12:59:27.140: Manager.vala:63: add_path
** (wingpanel:20895): DEBUG: 12:59:27.303: Volume-control.vala:656: Setting volume to 0,379974 for profile -1 because 0

You see a mix of:

  • DEBUG for the ones without log domain
  • wingpanel-DEBUG for wingpanel itself
  • wingpanel-indicator-bluetooth-DEBUG for bluetooth indicator.

I'm not sure what the correct RDNN is here, I see two different ones:

  • io.elementary.wingpanel.bluetooth.gresource.xml
  • io.elementary.desktop.wingpanel.bluetooth.gschema.xml

But that would create logs with prefixes like:

  • io.elementary.wingpanel.bluetooth-DEBUG
  • io.elementary.desktop.wingpanel.bluetooth-DEBUG

I do understand that RDNN will make it more scalable & future proof. Especially when retrieving logs from the whole OS. Update: It's probably also more discoverability.

@tintou
Copy link
Member

tintou commented Mar 19, 2019

I'd even prefer using https://gitlab.gnome.org/GNOME/vala/issues/765 once it is solved

@tintou tintou removed their request for review March 19, 2019 17:34
@danirabbit
Copy link
Member

Closing due to Corentin's comments

@danirabbit danirabbit closed this Mar 26, 2019
@peteruithoven
Copy link
Collaborator Author

I'm afraid I'm seeing very little progress on the Gnome / Vala side of things.
I might not understand that open issue 765 correctly but that seems to enable log domains per file, not per project? Which isn't really the use case I'm trying to solve.

I'm totally open for switching to RDNN.
Which "only" leaves the issue that this would need to be implemented in every project. One counter argument is that this can happen incrementally, you can add it when you need it per indicator / plug.

@peteruithoven peteruithoven reopened this Jun 13, 2019
@jeremypw
Copy link
Collaborator

jeremypw commented May 6, 2021

@peteruithoven Converting to draft as there are conflicts and no recent activity. Has anything changed since this was last touched? Is it still needed?

@jeremypw jeremypw marked this pull request as draft May 6, 2021 11:31
@peteruithoven
Copy link
Collaborator Author

I mean... again no activity in that gnome solution and I think this is still as relevant. Reading logs from indicators is still very hard.

@jeremypw
Copy link
Collaborator

jeremypw commented May 6, 2021

OK, if you fix the conflicts I'll look at it - it doesn't appear to do anything irreversible so I am not sure what the harm in merging it while waiting for a better solution is.

@peteruithoven
Copy link
Collaborator Author

I've fixed the merge requests.
Note, it follows RDNN using io.elementary.wingpanel.bluetooth.

@peteruithoven peteruithoven marked this pull request as ready for review May 8, 2021 10:18
Copy link
Collaborator

@jeremypw jeremypw left a comment

Choose a reason for hiding this comment

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

LGTM and works as expected.

@peteruithoven
Copy link
Collaborator Author

Thanks for the review!
Now we only need to do the same for all the other indicators :P

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.

None yet

4 participants