git-ecosystem / trace2receiver

GOLANG based OTEL custom collector receiver component for Trace2 data
MIT License
44 stars 7 forks source link

Clarity about detail levels #19

Closed hannahchiodo-msft closed 7 months ago

hannahchiodo-msft commented 8 months ago

I have noticed that once I introduced a filter.yml file limiting detail to "dl:summary", I no longer see git:remote-https commands in my telemetry data. Based on this section about detail levels, "dl:summary" is the default.

  1. Are network calls e.g. git:remote-https intended to be filtered out at "dl:summary"?
  2. Why was I getting telemetry for those commands before if the default was also "dl:summary"?
  3. Is there a guide somewhere to what information/commands are reported at each detail level?

Thank you!

hannahchiodo-msft commented 8 months ago

I figured out the cause. We added

defaults:
  ruleset: "dl:drop"

to drop all data that is not from our nicknamed repo. However, the result is remote-https, gvfs-helper, and other interesting subcommands are dropped, even when command is run from the named repo. Changing the default to "dl:summary" brings that data back. I don't believe this is the intended behavior.

jeffhostetler commented 8 months ago

Have you tried adding a custom ruleset yml file for your nicknamed repos. Let the ruleset yml govern all of the filtering/detail level for all commands from your important repos. Then leave let the defaults in the filter.yml drop all data from "non interesting" repos.

https://github.com/git-ecosystem/trace2receiver/blob/main/Docs/config-filter-settings.md#repo-nicknames https://github.com/git-ecosystem/trace2receiver/blob/main/Docs/config-ruleset-definition.md

For example, in https://github.com/git-ecosystem/trace2receiver/blob/main/Docs/Examples/Filters/20-Advanced/filter.yml we define a "monorepo" nickname and use ruleset "rs:monorepo" and that is defined in "rs-monorepo.yml" where we select the 4 commands for which we want verbose details. For other repos in this example, all commands are routed thru "rs-other.yml" (either by nickname or by fall-thru default), but you could just "dl:drop" them in the "filter.yml" "defaults:" section (and omit the "rs:other" ruleset completely if you wanted).

Hope this helps. Let me know if you have questions.

hannahchiodo-msft commented 8 months ago

Hi Jeff, that is how we currently are set up. Our filter.yml (with an added comment):

keynames:
  nickname_key: "otel.trace2.nickname"

nicknames:
  "<nickname>": "rs:<nickname>"

rulesets:
  "rs:<nickname>": "<path to ruleset file>"

defaults:
  ruleset: "dl:drop" # but if I change this to "dl:summary", I get the remote-https events again!

and our ruleset file has:

default:
  detail: "dl:summary" # even if I change this to "dl:verbose", I don't get remote-https events
jeffhostetler commented 8 months ago

@hannahchiodo-msft Let me take a look. I'll be out of the office the rest of the week, but I'll get back to you next week. Could you privately email me your YML files and I'll try to repro it. Thanks!

jeffhostetler commented 8 months ago

I found a problem in core Git that is causing this problem. git-remote-https.exe (and the other git-remote-*.exe executables) are build from a different main program (remote-curl.c rather than git.c) and the calls to trace2_cmd_list_config() and trace2_cmd_list_env_vars() are only in git.c.

So git-remote-https.exe is not sending the def_param events in the telemetry stream which is how the nickname is sent to the daemon. So the daemon is not seeing a nickname associated with the command and treating it like came from an uninteresting repo.

It's a two line fix in Git. I'll log it, but I'm not sure when I can work this into my schedule.

If anyone wants to give it a try, compare the following: https://github.com/git/git/blob/master/git.c#L464-L466 https://github.com/git/git/blob/master/remote-curl.c#L1551

(For extra credit, maybe also look at the other call sites of trace2_cmd_name() in git.c and see if any of them should be similarly updated -- I only did a quick scan and didn't trace the callers to see if they might be missing from some control flows.)

jeffhostetler commented 8 months ago

See: https://github.com/gitgitgadget/git/pull/1679

jeffhostetler commented 7 months ago

The Git problem has been fixed in v2.44.0.vfs.0.1 Please try that or a later version.

https://github.com/microsoft/git/releases