test: updated tests to be atomic

.github/workflows/ci-workflow.yml

@@ -5,116 +5,7 @@ on: [push, pull_request, workflow_dispatch]
 
 
 jobs:
-  skip_if_release:
-    runs-on: ubuntu-latest
-
-    outputs:
-      should_skip: ${{ steps.skip_check.outputs.should_skip }}
-
-    steps:
-      - id: skip_check
-        uses: fkirc/skip-duplicate-actions@v5
-        with:
-          paths_ignore: '["NEWS.md", "changelog.json", "package.json", "package-lock.json"]'
-          skip_after_successful_duplicate: false
-          do_not_skip: '["workflow_dispatch", "pull_request"]'
-
-  lint:
-    needs: skip_if_release
-    if: needs.skip_if_release.outputs.should_skip != 'true'
-    runs-on: ubuntu-latest
-
-    strategy:
-      matrix:
-        node-version: [lts/*]
-
-    steps:
-    - uses: actions/checkout@v3
-    - name: Use Node.js ${{ matrix.node-version }}
-      uses: actions/setup-node@v3
-      with:
-        node-version: ${{ matrix.node-version }}
-    - name: Install Dependencies
-      run: npm ci
-    - name: Run Linting
-      run: npm run lint
-    - name: Inspect Lockfile
-      run: npm run lint:lockfile
-
-  ci:
-    needs: skip_if_release
-    if: needs.skip_if_release.outputs.should_skip != 'true'
-    runs-on: ubuntu-latest
-
-    strategy:
-      matrix:
-        node-version: [lts/*]
-
-    steps:
-    - uses: actions/checkout@v3
-    - name: Use Node.js ${{ matrix.node-version }}
-      uses: actions/setup-node@v3
-      with:
-        node-version: ${{ matrix.node-version }}
-    - name: Install Dependencies
-      run: npm ci
-    - name: Run CI Script Unit Tests
-      run: npm run unit:scripts
-
-  unit:
-    needs: skip_if_release
-    if: needs.skip_if_release.outputs.should_skip != 'true'
-    runs-on: ubuntu-latest
-
-    strategy:
-      matrix:
-        node-version: [16.x, 18.x, 20.x]
-
-    steps:
-    - uses: actions/checkout@v3
-    - name: Use Node.js ${{ matrix.node-version }}
-      uses: actions/setup-node@v3
-      with:
-        node-version: ${{ matrix.node-version }}
-    - name: Install Dependencies
-      run: npm ci
-    - name: Run Unit Tests
-      run: npm run unit
-    - name: Archive Unit Test Coverage
-      uses: actions/upload-artifact@v3
-      with:
-        name: unit-tests-${{ matrix.node-version }}
-        path: ./coverage/unit/lcov.info
-  integration:
-    needs: skip_if_release
-    if: needs.skip_if_release.outputs.should_skip != 'true'
-    runs-on: ubuntu-latest
-
-    strategy:
-      matrix:
-        node-version: [16.x, 18.x, 20.x]
-
-    steps:
-    - uses: actions/checkout@v3
-    - name: Use Node.js ${{ matrix.node-version }}
-      uses: actions/setup-node@v3
-      with:
-        node-version: ${{ matrix.node-version }}
-    - name: Install Dependencies
-      run: npm ci
-    - name: Run Docker Services
-      run: npm run services
-    - name: Run Integration Tests
-      run: npm run integration
-    - name: Archive Integration Test Coverage
-      uses: actions/upload-artifact@v3
-      with:
-        name: integration-tests-${{ matrix.node-version }}
-        path: ./coverage/integration/lcov.info
-
   versioned-internal:
-    needs: skip_if_release
-    if: needs.skip_if_release.outputs.should_skip != 'true'
     runs-on: ubuntu-latest
 
     strategy:
@@ -134,65 +25,7 @@ jobs:
     - name: Run Versioned Tests
       run: TEST_CHILD_TIMEOUT=600000 npm run versioned:internal
       env:
-        VERSIONED_MODE: ${{ github.ref == 'refs/heads/main' && '--minor' || '--major' }}
+        VERSIONED_MODE: --minor 
         JOBS: 4 # 2 per CPU seems to be the sweet spot in GHA (July 2022)
         C8_REPORTER: lcovonly
-    - name: Archive Versioned Test Coverage
-      uses: actions/upload-artifact@v3
-      with:
-        name: versioned-tests-${{ matrix.node-version }}
-        path: ./coverage/versioned/lcov.info
 
-  # There is no coverage for external as that's tracked in their respective repos
-  versioned-external:
-    needs: skip_if_release
-    if: needs.skip_if_release.outputs.should_skip != 'true'
-    runs-on: ubuntu-latest
-
-    strategy:
-      matrix:
-        node-version: [16.x, 18.x, 20.x]
-
-    steps:
-    - uses: actions/checkout@v3
-    - name: Use Node.js ${{ matrix.node-version }}
-      uses: actions/setup-node@v3
-      with:
-        node-version: ${{ matrix.node-version }}
-    - name: Install Dependencies
-      run: npm ci
-    - name: Run Versioned Tests
-      run: TEST_CHILD_TIMEOUT=600000 npm run versioned:external
-      env:
-        VERSIONED_MODE: ${{ github.ref == 'refs/heads/main' && '--minor' || '--major' }}
-        JOBS: 4 # 2 per CPU seems to be the sweet spot in GHA (July 2022)
-  codecov:
-    needs: [unit, integration, versioned-internal]
-    runs-on: ubuntu-latest
-
-    strategy:
-      matrix:
-        node-version: [16.x, 18.x, 20.x]
-
-    steps:
-      - uses: actions/checkout@v3
-      - name: Download artifacts
-        uses: actions/download-artifact@v3
-      - name: Post Unit Test Coverage
-        uses: codecov/codecov-action@v3
-        with:
-          token: ${{ secrets.CODECOV_TOKEN }}
-          directory: unit-tests-${{ matrix.node-version }}
-          flags: unit-tests-${{ matrix.node-version }}
-      - name: Post Integration Test Coverage
-        uses: codecov/codecov-action@v3
-        with:
-          token: ${{ secrets.CODECOV_TOKEN }}
-          directory: integration-tests-${{ matrix.node-version }}
-          flags: integration-tests-${{ matrix.node-version }}
-      - name: Post Versioned Test Coverage
-        uses: codecov/codecov-action@v3
-        with:
-          token: ${{ secrets.CODECOV_TOKEN }}
-          directory: versioned-tests-${{ matrix.node-version }}
-          flags: versioned-tests-${{ matrix.node-version }}

bin/docker-services.sh

@@ -48,6 +48,9 @@ else
   docker run -d --name nr_node_elastic \
   -p 9200:9200 \
   -e "discovery.type=single-node" \
+  -e "http.host=0.0.0.0" \
+  -e "network.host=_site_" \
+  -e "ES_JAVA_OPTS=-Xms128m -Xmx128m" \
   -e "xpack.security.enabled=false" \
   docker.elastic.co/elasticsearch/elasticsearch:8.8.2;
 fi

test/lib/params.js

@@ -28,8 +28,6 @@ module.exports = {
 
   elastic_host: process.env.NR_NODE_TEST_ELASTIC_HOST || 'localhost',
   elastic_port: process.env.NR_NODE_TEST_ELASTIC_PORT || 9200,
-  elastic_user: process.env.NR_NODE_TEST_ELASTIC_USER || 'elastic',
-  elastic_pass: process.env.NR_NODE_TEST_ELASTIC_PASS || 'changeme',
 
   postgres_host: process.env.NR_NODE_TEST_POSTGRES_HOST || 'localhost',
   postgres_port: process.env.NR_NODE_TEST_POSTGRES_PORT || 5432,

test/versioned/elastic/elasticsearch.tap.js

@@ -10,19 +10,24 @@ const test = tap.test
 const helper = require('../../lib/agent_helper')
 const params = require('../../lib/params')
 const urltils = require('../../../lib/util/urltils')
+const crypto = require('crypto')
+const DB_INDEX = `test-${randomString()}`
+const DB_INDEX_2 = `test2-${randomString()}`
 
-test('Elasticsearch instrumentation', { timeout: 20000 }, (t) => {
+function randomString() {
+  return crypto.randomBytes(5).toString('hex')
+}
+
+test('Elasticsearch instrumentation', (t) => {
   t.autoend()
 
   let METRIC_HOST_NAME = null
   let HOST_ID = null
-  const DB_INDEX = `test`
-  const DB_INDEX_2 = `test2`
 
   let agent
   let client
 
-  t.beforeEach(async () => {
+  t.before(async () => {
     agent = helper.instrumentMockedAgent()
 
     METRIC_HOST_NAME = urltils.isLocalhost(params.elastic_host)
@@ -33,37 +38,50 @@ test('Elasticsearch instrumentation', { timeout: 20000 }, (t) => {
     // need to capture attributes
     agent.config.attributes.enabled = true
 
-    const { Client, HttpConnection } = require('@elastic/elasticsearch')
+    const { Client } = require('@elastic/elasticsearch')
     client = new Client({
       node: `http://${params.elastic_host}:${params.elastic_port}`,
-      auth: {
-        username: params.elastic_user,
-        password: params.elastic_pass
-      },
-      Connection: HttpConnection
+      maxRetries: 5,
+      sniffOnStart: true,
+      log: 'error'
     })
+
+    console.log('before ping')
+    await client.ping()
+    console.log('after ping')
+    console.log('before first idx')
+    await client.indices.create({ index: DB_INDEX })
+    console.log('after first idx')
+    console.log('before 2nd idx')
+    // await client.indices.create({ index: DB_INDEX_2 })
   })
 
-  t.afterEach(async () => {
-    agent && helper.unloadAgent(agent)
+  t.afterEach(() => {
+    agent.queries.clear()
   })
 
-  t.teardown(async () => {
-    await client.indices.delete({ index: DB_INDEX })
-    await client.indices.delete({ index: DB_INDEX_2 })
+  t.teardown(() => {
+    agent && helper.unloadAgent(agent)
+    return Promise.all([
+      client.indices.delete({ index: DB_INDEX }),
+      client.indices.delete({ index: DB_INDEX_2 })
+    ])
   })
 
   t.test('should be able to record creating an index', async (t) => {
+    const index = `test-index-${randomString()}`
+    t.teardown(async () => {
+      await client.indices.delete({ index })
+    })
     await helper.runInTransaction(agent, async function transactionInScope(transaction) {
       t.ok(transaction, 'transaction should be visible')
-      await client.indices.create({ index: DB_INDEX })
-      await client.indices.create({ index: DB_INDEX_2 })
+      await client.indices.create({ index })
       const trace = transaction.trace
       t.ok(trace?.root?.children?.[0], 'trace, trace root, and first child should exist')
       const firstChild = trace.root.children[0]
       t.equal(
         firstChild.name,
-        'Datastore/statement/ElasticSearch/test/index.create',
+        `Datastore/statement/ElasticSearch/${index}/index.create`,
         'should record index PUT as create'
       )
     })
@@ -111,22 +129,21 @@ test('Elasticsearch instrumentation', { timeout: 20000 }, (t) => {
     agent.config.slow_sql.enabled = true
     await helper.runInTransaction(agent, async function transactionInScope(transaction) {
       const expectedQuery = { q: 'sixth' }
-      const search = await client.search({ index: DB_INDEX_2, q: 'sixth' })
+      const search = await client.search({ index: DB_INDEX_2, ...expectedQuery })
       t.ok(search, 'search should return a result')
       t.ok(transaction, 'transaction should still be visible after search')
       const trace = transaction.trace
       t.ok(trace?.root?.children?.[0], 'trace, trace root, and first child should exist')
       const firstChild = trace.root.children[0]
       t.match(
         firstChild.name,
-        'Datastore/statement/ElasticSearch/test2/search',
+        `Datastore/statement/ElasticSearch/${DB_INDEX_2}/search`,
         'querystring search should be recorded as a search'
       )
       const attrs = firstChild.getAttributes()
       t.match(attrs.product, 'ElasticSearch')
       t.match(attrs.host, METRIC_HOST_NAME)
       transaction.end()
-      // can we inspect recorded query?
       t.ok(agent.queries.samples.size > 0, 'there should be a query sample')
       for (const query of agent.queries.samples.values()) {
         t.ok(query.total > 0, 'the samples should have positive duration')
@@ -153,14 +170,16 @@ test('Elasticsearch instrumentation', { timeout: 20000 }, (t) => {
       const firstChild = trace.root.children[0]
       t.match(
         firstChild.name,
-        'Datastore/statement/ElasticSearch/test/search',
+        `Datastore/statement/ElasticSearch/${DB_INDEX}/search`,
         'search index is specified, so name shows it'
       )
       const attrs = firstChild.getAttributes()
-      t.match(attrs.product, 'ElasticSearch')
-      t.match(attrs.host, METRIC_HOST_NAME)
+      t.equal(attrs.product, 'ElasticSearch')
+      t.equal(attrs.host, METRIC_HOST_NAME)
+      t.equal(attrs.port_path_or_id, `${params.elastic_port}`)
+      // TODO: update once instrumentation is properly setting database name
+      t.equal(attrs.database_name, 'unknown')
       transaction.end()
-      // can we inspect recorded query?
       t.ok(agent.queries.samples.size > 0, 'there should be a query sample')
       for (const query of agent.queries.samples.values()) {
         t.ok(query.total > 0, 'the samples should have positive duration')
@@ -195,7 +214,6 @@ test('Elasticsearch instrumentation', { timeout: 20000 }, (t) => {
       t.match(attrs.product, 'ElasticSearch')
       t.match(attrs.host, METRIC_HOST_NAME)
       transaction.end()
-      // can we inspect recorded query?
       t.ok(agent.queries.samples.size > 0, 'there should be a query sample')
       for (const query of agent.queries.samples.values()) {
         t.ok(query.total > 0, 'the samples should have positive duration')
@@ -260,22 +278,23 @@ test('Elasticsearch instrumentation', { timeout: 20000 }, (t) => {
   })
 
   t.test('should create correct metrics', async function (t) {
+    const id = `key-${randomString()}`
     t.plan(28)
     await helper.runInTransaction(agent, async function transactionInScope(transaction) {
       await client.index({
         index: DB_INDEX,
-        id: 'testkey2',
+        id,
         document: {
           title: 'second document',
           body: 'body of the second document'
         }
       })
 
       // check metrics/methods for "exists" queries
-      await client.exists({ id: 'testkey2', index: DB_INDEX })
-      await client.get({ id: 'testkey2', index: DB_INDEX })
+      await client.exists({ id, index: DB_INDEX })
+      await client.get({ id, index: DB_INDEX })
       await client.search({ query: { match: { body: 'document' } } })
-      await client.delete({ id: 'testkey2', index: DB_INDEX })
+      await client.delete({ id, index: DB_INDEX })
       transaction.end()
 
       const unscoped = transaction.metrics.unscoped
@@ -288,10 +307,10 @@ test('Elasticsearch instrumentation', { timeout: 20000 }, (t) => {
         'Datastore/operation/ElasticSearch/doc.get': 1,
         'Datastore/operation/ElasticSearch/doc.exists': 1,
         'Datastore/operation/ElasticSearch/search': 1,
-        'Datastore/statement/ElasticSearch/test/doc.create': 1,
-        'Datastore/statement/ElasticSearch/test/doc.get': 1,
-        'Datastore/statement/ElasticSearch/test/doc.exists': 1,
-        'Datastore/statement/ElasticSearch/test/doc.delete': 1,
+        [`Datastore/statement/ElasticSearch/${DB_INDEX}/doc.create`]: 1,
+        [`Datastore/statement/ElasticSearch/${DB_INDEX}/doc.get`]: 1,
+        [`Datastore/statement/ElasticSearch/${DB_INDEX}/doc.exists`]: 1,
+        [`Datastore/statement/ElasticSearch/${DB_INDEX}/doc.delete`]: 1,
         'Datastore/statement/ElasticSearch/any/search': 1
       }
       expected['Datastore/instance/ElasticSearch/' + HOST_ID] = 5