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

feat: improve SQL summarization to match cross-agent spec #2881

Draft
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

trentm
Copy link
Member

@trentm trentm commented Aug 16, 2022

The cross-agent APM spec defines test cases for SQL statement
summarization used for 'span.name'
https://github.com/elastic/apm/blob/main/tests/agents/json-specs/sql_signature_examples.json

status / motivation

Currently the Node.js APM agent does not conform to the cross-agent SQL signature/summarization spec (see the test summary below). That spec was initially added because it was shared across the Go and Ruby agents. There isn't a specific user request to get the Node.js APM agent to conform. The only motivation is to be up to date on cross-agent specs. tl;dr: This is currently low priority.

So far this PR just adds a test file that shows where the Node.js APM agent currently fails spec.

checklist

  • Update the Node.js APM agent's sql summarization to match spec, or discuss and skip some of these tests.

initial test state

Currently the Node.js agent passes 18 and fails 17 of the sql_signature_examples cases.

% node test/sql-signature.test.js | rg "(not | expected:| actual:|^#|^\s*')"  | cat
# cross-agent SQL statement summary/signature
not ok 5 "SELECT * FROM `foo.bar`"
    expected: 'SELECT FROM foo.bar'
    actual:   'SELECT FROM `foo.bar`'
not ok 6 "SELECT * FROM \"foo.bar\""
    expected: 'SELECT FROM foo.bar'
    actual:   'SELECT FROM "foo.bar"'
not ok 7 "SELECT * FROM [foo.bar]"
    expected: 'SELECT FROM foo.bar'
    actual:   'SELECT FROM [foo.bar]'
not ok 8 "SELECT (x, y) FROM foo,bar,baz"
    expected: 'SELECT FROM foo'
    actual:   'SELECT FROM foo,bar,baz'
not ok 11 "SELECT id FROM \"myta\n-æøåble\" WHERE id = 2323"
    expected: |-
      'SELECT FROM myta\n-æøåble'
    actual: |-
      'SELECT FROM "myta'
not ok 12 "SELECT * FROM foo-- abc\n./*def*/bar"
    expected: |-
      'SELECT FROM foo.bar'
    actual: |-
      'SELECT FROM foo--'
not ok 13 "SELECT *,(SELECT COUNT(*) FROM table2 WHERE table2.field1 = table1.id) AS count FROM table1 WHERE table1.field1 = 'value'" # We capture the first table of the outermost select statement
    expected: 'SELECT FROM table1'
    actual:   'SELECT FROM table2'
not ok 14 "SELECT * FROM (SELECT foo FROM bar) AS foo_bar" # If the outermost select operates on derived tables, then we just return 'SELECT' (i.e. the fallback)
    expected: 'SELECT'
    actual:   'SELECT FROM ('
not ok 16 "UPDATE IGNORE foo.bar SET bar=1 WHERE baz=2"
    expected: 'UPDATE foo.bar'
    actual:   'UPDATE IGNORE'
not ok 17 "UPDATE ONLY foo AS bar SET baz=1"
    expected: 'UPDATE foo'
    actual:   'UPDATE ONLY'
not ok 20 "CALL foo(bar, 123)"
    expected: 'CALL foo'
    actual:   'CALL'
not ok 21 "ALTER TABLE foo ADD ()" # For DDL we only capture the first token
    expected: 'ALTER'
    actual:   'ALTER TABLE foo'
not ok 22 "CREATE TABLE foo ..."
    expected: 'CREATE'
    actual:   'CREATE TABLE foo'
not ok 23 "DROP TABLE foo"
    expected: 'DROP'
    actual:   'DROP TABLE foo'
not ok 28 "SELECT * FROM (SELECT EOF" # For broken statements we only capture the first token
    expected: 'SELECT'
    actual:   'SELECT FROM ('
not ok 29 "SELECT 'neverending literal FROM (SELECT * FROM ..."
    expected: 'SELECT'
    actual:   'SELECT FROM ('
not ok 32 "UPDATE 99"
    expected: 'UPDATE'
    actual:   'UPDATE 99'
# tests 35
# pass  18
# fail  17

@trentm trentm self-assigned this Aug 16, 2022
@github-actions github-actions bot added the agent-nodejs Make available for APM Agents project planning. label Aug 16, 2022
@apmmachine
Copy link
Contributor

💔 Build Failed

the below badges are clickable and redirect to their specific view in the CI or DOCS
Pipeline View Test View Changes Artifacts preview preview

Expand to view the summary

Build stats

  • Start Time: 2022-08-16T17:29:57.896+0000

  • Duration: 14 min 50 sec

Steps errors 68

Expand to view the steps failures

Show only the first 10 steps failures

Run Tests
  • Took 1 min 25 sec . View more details here
  • Description: .ci/scripts/test.sh -b "release" -t "" "8"
Archive the artifacts
  • Took 0 min 0 sec . View more details here
  • Description: [2022-08-16T17:43:12.722Z] Archiving artifacts hudson.AbortException: script returned exit code 1
Run Tests
  • Took 3 min 10 sec . View more details here
  • Description: .ci/scripts/test.sh -b "release" -t "" "8.6"
Run Tests
  • Took 1 min 29 sec . View more details here
  • Description: .ci/scripts/test.sh -b "release" -t "" "8.6"
Archive the artifacts
  • Took 0 min 0 sec . View more details here
  • Description: [2022-08-16T17:43:38.781Z] Archiving artifacts hudson.AbortException: script returned exit code 1
Run Tests
  • Took 3 min 6 sec . View more details here
  • Description: .ci/scripts/test.sh -b "release" -t "" "8.6"
Run Tests
  • Took 1 min 28 sec . View more details here
  • Description: .ci/scripts/test.sh -b "release" -t "" "8.6"
Archive the artifacts
  • Took 0 min 0 sec . View more details here
  • Description: [2022-08-16T17:43:36.922Z] Archiving artifacts hudson.AbortException: script returned exit code 1
Windows Batch Script
  • Took 1 min 1 sec . View more details here
  • Description: node test/test.js
Error signal
  • Took 0 min 0 sec . View more details here
  • Description: hudson.AbortException: script returned exit code 1

🤖 GitHub comments

To re-run your PR in the CI, just comment with:

  • /test : Re-trigger the build.

  • run module tests for <modules> : Run TAV tests for one or more modules, where <modules> can be either a comma separated list of modules (e.g. memcached,redis) or the string literal ALL to test all modules

  • run benchmark tests : Run the benchmark test only.

  • run elasticsearch-ci/docs : Re-trigger the docs validation. (use unformatted text in the comment!)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
agent-nodejs Make available for APM Agents project planning.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants