search

getappmap / appmap-java

AppMap client agent for Java
Other
80 stars 14 forks source link

Recording of getters is configurable #212

Open kgilpin opened 1 year ago

kgilpin commented 1 year ago

The agent currently assumes that methods that start with get.*, is.*, and has.* are getters of some sort, and so are not interesting enough to record.

This assumption isn't valid in some applications, so it should possible to disable it.

kgilpin commented 1 year ago

@apotterri should recording of getters and setters be disabled by default? In Ruby and Python we don't record field access and no one seems to care.

apotterri commented 1 year ago

Methods that start with get and set are currently ignored. The problem with this is that they might not be getters and setters (since naming methods this way is just a convention).

I originally wrote this issue because some non-getter methods with names that started with get were missing from an AppMap I was looking at. TBH, I don't remember whether that recording was from a user's app, or something I stumbled on.

apotterri commented 11 months ago

Huh, actually looks like you wrote this issue. :) Maybe I meant to and forgot....

apotterri commented 7 months ago

This behavior trips users up all the time: https://appmap-group.slack.com/archives/C03GZSGT41X/p1707477689987499?thread_ts=1707476013.248799&cid=C03GZSGT41X .

I think it actually might be better to not only make the recording configurable, but make the default be to record getters. The user could then disable recording if the getters blow up their recordings.

@kgilpin WDYT?

kgilpin commented 7 months ago

To me, a getter is not only the naming pattern, but also the implementation. 

getUser() {
  return this.user;
}

That sort of thing.

apotterri commented 7 months ago

So, you think the agent should analyze the bytecode to see how complicated the method is? Do any of the other agents do that?

kgilpin commented 7 months ago

Well, in other languages this type of situation would usually be a field access rather than a function call, so it would not be recorded.

apotterri commented 7 months ago

Hmmm, yeah, ok.

So, I looked at the bytecode for Lombok's @Getter methods, Java records (available in Java 14+), and the code generated for a method like the one you wrote. They're all exactly the same three instructions, so it seems like it should be (relatively) straightforward to detect methods like those and ignore them.

I'd suggest that is* and has* methods aren't really a thing in Java. Maybe we can just drop detecting these?