Skip to content
This repository was archived by the owner on Oct 31, 2024. It is now read-only.

feat: new elasticsearch instrumentation #70

Merged
merged 30 commits into from
Mar 4, 2021
Merged

feat: new elasticsearch instrumentation #70

merged 30 commits into from
Mar 4, 2021

Conversation

YanivD
Copy link
Contributor

@YanivD YanivD commented Mar 1, 2021

No description provided.

@YanivD YanivD requested a review from blumamir March 1, 2021 13:31
"@opentelemetry/api": "^0.17.0",
"@opentelemetry/instrumentation": "^0.17.0",
"@opentelemetry/semantic-conventions": "^0.17.0",
"test-all-versions": "^5.0.1"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be dev dependency

import { startSpan, onError, onResponse, defaultDbStatementSerializer, normalizeArguments } from './utils';
import { ELASTICSEARCH_API_FILES } from './helpers';

type Config = InstrumentationConfig & ElasticsearchInstrumentationConfig;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i actually changes this and used ElasticsearchInstrumentationConfig directly instead

import { Span } from '@opentelemetry/api';
import { InstrumentationConfig } from '@opentelemetry/instrumentation';

export interface SerializerPayload {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

unused, copied from mongoose

aggregatePipeline?: any;
}

export type DbStatementSerializer = (params?: object, options?: object) => string;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

could be nice to pass the operation as well, like in mongoose and redis

@@ -0,0 +1,8 @@
Copyright 2021 @ Aspecto
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

use apache license

"watch": "tsc -w",
"version:update": "node ../../scripts/version-update.js",
"version": "yarn run version:update",
"test": "mocha"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

shouldn't be invoked with tav?

package.json Outdated
@@ -3,6 +3,7 @@
"private": true,
"scripts": {
"test": "lerna run test",
"test:ci": "yarn test",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

lerna run test:ci

package.json Outdated
@@ -2,7 +2,7 @@
"name": "root",
"private": true,
"scripts": {
"test": "lerna run test",
"test": "lerna run test:ci",
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

should be lerna run test
and add another one called "test:ci": "lerna run test:ci"

@nirsky nirsky changed the title new elasticsearch instrumentation feat: new elasticsearch instrumentation Mar 1, 2021
@YanivD YanivD merged commit 08625e2 into master Mar 4, 2021
@YanivD YanivD deleted the feat/esInstument branch March 4, 2021 13:06
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants