[TrackEvent] Tweak category/tags filters priority

[etiennep-chromium]

This PR tweaks priority of category and tags filter to fit Chromium use cases:

• specific > pattern > single char "*" wildcard (see b/260418655)
• tags disabled > enabled: this is mostly useful for tags E.g. disabled_tags: "slow", enabled_tags: "navigation" Expectation: categories with "navigation" tags are enabled, but not if "slow" tag is specified.
• Don't disable slow/debug tags if there are any explicitly enabled tags: there's otherwise no way of 'not' disabled slow and debug at the tags level E.g. enabled_tags: "navigation" Expectation: categories with "navigation" tags are enabled, including ones with the "slow" tag We can't specify enabled_tags: "slow" because that would enable all "slow" categories.

So before logic ranks (in this order):

• specific names > patterns
• enabled > disabled
• categories > tags

After logic ranks:

• specific > pattern > single char "*" wildcard
• categories (enabled > disabled) > tags (disabled > enabled)

Bug: chromium:260418655

[betasheet]

I'm generally onboard with such a change, but I'm a bit scared if we are silently breaking some configs people already use. We would definitely have to announce this in the changelog, maybe do some form of audit of existing configs by major customers. @ddiproietto should take a look too :)

[betasheet]

So before logic ranks (in this order):

• specific names > patterns
• enabled > disabled
• categories > tags

After logic ranks:

• specific > pattern > single char "*" wildcard
• disabled > enabled
• categories > tags

I think this is actually also swapping the last two lines, i.e.

1. specific > pattern > single char "*" wildcard
2. categories > tags
3. disabled > enabled

[etiennep-chromium]

disabled_categories: "some_categories*", enabled_categories: "some_categories_i_want_*" Would mean some_categories_i_want_ categories stay disabled.

Expending on this: I don't have a strong opinion on how this should be interpreted, but I think the similar situation with tags I gave in the description (E.g. disabled_tags: "slow", enabled_tags: "navigation") should be interpreted as "navigation.debug" is disabled. Then, for consistency, I applied the same logic on categories. It would be easy to implement enabled categories > disabled categories > disabled tags > enabled tags But I doubt there's a real use case for this at the time.

[ddiproietto]

Thanks for doing this!

Useless complaint: this still doesn't give us perfect control over tracing categories. An explicit priority based approach (iptables style) woud be more powerful, but perhaps not worth it, due to the complexity and performance (I believe NameMatchesPattern has to be reasonably fast for dynamic categories). So this is fine.

W.R.T. the breaking change, I think chromium is the more advanced user of this. (we cannot know all the users and all their configs, though). After review, I would announce the change on https://groups.google.com/g/perfetto-dev, and give people one week to complain. If we have no complaints, I would go ahead with this.

[ddiproietto]

If you want to pile up another breaking change, I was thinking we could disable all categories that don't match anything by default. We had some discussions a year ago and people were generally on board. (totally up to you if you want to do this separately).

[ddiproietto]

Also, can I ask you to check in a test like:

  auto* tracing_session = NewTraceWithCategories({"foo"});
  EXPECT_TRUE(TRACE_EVENT_CATEGORY_ENABLED("foo"));

that covers most of the scenarios in the bug, BEFORE submitting this PR? That way we can check what's changing?

Thanks!

[etiennep-chromium]

If you want to pile up another breaking change, I was thinking we could disable all categories that don't match anything by default

I tried this out and it showed a lot of tests in perfetto need updating (they use the empty track event config). I'm a bit opposed to piling up this change because there's lower benefit (it's easy to add disable_categories = [*] everywhere), and risks blocking the API change I want to get in in this CL.

Also, can I ask you to check in a test like:

Done here: https://github.com/google/perfetto/pull/1484/files

[etiennep-chromium]

After review, I would announce the change on https://groups.google.com/g/perfetto-dev, and give people one week to complain. If we have no complaints, I would go ahead with this.

Announce sent and CHANGELOG updated, PTAnL?