apm-agent-python icon indicating copy to clipboard operation
apm-agent-python copied to clipboard

Update SQL parser to parse all shared tests

Open felixbarny opened this issue 5 years ago • 5 comments

This parser should parse the following test cases:

https://github.com/elastic/apm/blob/master/tests/agents/json-specs/sql_signature_examples.json https://github.com/elastic/apm/blob/master/tests/agents/json-specs/sql_token_examples.json

Refers to https://github.com/elastic/apm/issues/13

felixbarny avatar Sep 08 '20 06:09 felixbarny

@felixbarny should we move those test cases to https://github.com/elastic/apm/tree/master/tests/agents/json-specs? I can open a PR

/edit: turns out they are already there, I was confused by the links to apm-agent-go

beniwohli avatar Sep 08 '20 06:09 beniwohli

Whoops, that was a copy/paste error. I've updated the links.

felixbarny avatar Sep 08 '20 09:09 felixbarny

One could argue that the token examples are an implementation detail and not strictly a requirement?

mikker avatar Sep 08 '20 10:09 mikker

Unfortunately, our current parser fails about half of the signature tests. I'll look into adapting the parser from the Go agent.

beniwohli avatar Sep 08 '20 10:09 beniwohli

The original Ruby implementation was also very far from living up to the examples. FWIW copying Andrew's Go implementation to Ruby was a fun challenge and the examples really helped.

https://github.com/elastic/apm-agent-ruby/pull/640

When it was done, we released it as an opt-in feature. Then after a while we flipped the defaults, making it opt-in to use the old implementation. Finally we removed the old one. Never heard anything from users which I take as a great success.

The new implementation was slower than the original but actually usable with anything but the simplest queries.

mikker avatar Sep 08 '20 10:09 mikker