Origo: [Argus] Opentelemetry instrumentation for better metrics & tracing #4779

zeeshanakram3 · 2023-06-01T18:32:28Z

Addresses #4763

vercel · 2023-06-01T18:32:31Z

The latest updates on your projects. Learn more about Vercel for Git ↗︎

1 Ignored Deployment

Name	Status	Preview	Updated (UTC)
pioneer-testnet	⬜️ Ignored (Inspect)		Jul 14, 2023 10:32am

zeeshanakram3 · 2023-06-02T04:27:43Z

How to Test

Setup Elastic search stack (Elasticsearch + Kibana + APM server). A utility script (./start-elasticsearch-stack.sh) has been provided. It will automatically bootstrap all the relevant services & set up all the necessary configurations.
After bootstraping the ES Stack using ./start-elasticsearch-stack.sh. go to Kibana dashboard at http://localhost:5601/ and login with following credentials:
- username=elastic
- password=password
Go to Kibana APM integration endpoint, and click on Add Elastic APM. Then click on Save and Continue
Next run any application instrumented with OpenTelemetry protocol. For reference, distributor-1 service from the monorepo root docker-compse.yml file has been configured to export trace logs to the Elasticsearch server(see the configuration variables needed to be exported).
After the application is up & running, go to Traces endpoint in the Kibana dashboard, and you should be able to see the trace logs for different requests. e.g., below is a trace for GET api/v1/assets/:id request to Argus

zeeshanakram3 · 2023-06-02T04:41:08Z

@kdembler please test this PR based on the above instructions, and let me know whether it meets your requirements for monitoring purposes. Also, feel free to ping me if there is any issue setting up the infra

kdembler · 2023-06-02T17:45:33Z

@zeeshanakram3 This looks lovely! I will be happy to test it out, but I will be unavailable next week, will be back on 12th and will check it out as soon as possible

mnaamani · 2023-06-08T07:25:11Z

This looks like a really nice addition to provide visibility into how the application is performing.
I'm having issues however in getting the the distributor-1 to run successfully with instrumentation.

First observation, when starting the ES stack, the apm-server service name is coming up as apm-server-1

Secondly the distributor is crashing, with failure to validate the JOYSTREAM_DISTRIBUTOR__OTLP__ENDPOINT env variable.

I tried to change it from http://apm-server:8200 to "http://apm-server-1:8200" (with quotes) but it still fails with:

2023-06-08 10:49:47 $ node --require ./lib/app/instrumentation.js ./bin/run start
2023-06-08 10:49:51 Starting tracing...
2023-06-08 10:49:51 /joystream/distributor-node/lib/services/parsers/ConfigParserService.js:96
2023-06-08 10:49:51                     throw e;
2023-06-08 10:49:51                     ^
2023-06-08 10:49:51 
2023-06-08 10:49:51 ValidationError: Invalid env value of JOYSTREAM_DISTRIBUTOR__OTLP__ENDPOINT
2023-06-08 10:49:51 
2023-06-08 10:49:51 
2023-06-08 10:49:51     at ConfigParserService.setConfigEnvValue (/joystream/distributor-node/lib/services/parsers/ConfigParserService.js:89:27)
2023-06-08 10:49:51     at /joystream/distributor-node/lib/services/parsers/ConfigParserService.js:109:18
2023-06-08 10:49:51     at Array.forEach (<anonymous>)
2023-06-08 10:49:51     at ConfigParserService.mergeEnvConfigWith (/joystream/distributor-node/lib/services/parsers/ConfigParserService.js:104:14)
2023-06-08 10:49:51     at ConfigParserService.parse (/joystream/distributor-node/lib/services/parsers/ConfigParserService.js:133:14)
2023-06-08 10:49:51     at Object.<anonymous> (/joystream/distributor-node/lib/app/instrumentation.js:27:31)
2023-06-08 10:49:51     at Module._compile (internal/modules/cjs/loader.js:1114:14)
2023-06-08 10:49:51     at Object.Module._extensions..js (internal/modules/cjs/loader.js:1143:10)
2023-06-08 10:49:51     at Module.load (internal/modules/cjs/loader.js:979:32)
2023-06-08 10:49:51     at Function.Module._load (internal/modules/cjs/loader.js:819:12) {
2023-06-08 10:49:51   errors: [],
2023-06-08 10:49:51   errorMessages: []
2023-06-08 10:49:51 }
2023-06-08 10:49:51 error Command failed with exit code 1.

This is happening on Mac Desktop Docker..

I'll try on Linux and report back. But I suspect the same is happening on Linux and that is why the Full scenario is failing integration tests at job manage channels and videos through CLI: Failed

mnaamani · 2023-06-08T07:58:32Z

Even without starting with instrumentation, the same error is occuring:

2023-06-08 11:58:39 $ ./bin/run start
2023-06-08 11:57:34 
2023-06-08 11:57:34 
2023-06-08 11:57:34 error Command failed with exit code 1.
2023-06-08 11:57:45     Error: Invalid env value of JOYSTREAM_DISTRIBUTOR__OTLP__ENDPOINT
2023-06-08 11:57:45

Is there something perhaps wrong in the changes in configSchema.ts ?

mnaamani · 2023-06-08T08:31:50Z

Was able to resolve the problem by uncommenting the otlp section in the distributor-node/config.yml
I think this is because the properties are "required" so they must exist in the default config file, even if they are overriden by the env value.

mnaamani · 2023-06-08T09:00:46Z

Reverted to use http://apm-server:8200 endpoint, and was able to setup the APM integration and seeing

mnaamani · 2023-06-08T09:04:53Z

Interestingly enough, the tracing showed a potential problem with reading the package.json file in which the only code in distributor-node that does this is:

ConfigParserService.ts

public getNodeVersion(): string {
    const packageJSON = JSON.parse(fs.readFileSync(path.join(__dirname, '../../../package.json')).toString())
    return String(packageJSON.version)
  }

mnaamani · 2023-06-08T09:08:42Z

distributor-node/config.yml

@@ -19,6 +19,9 @@ logs:
  #   auth:
  #     username: username
  #     password: password
+# otlp:


This needs to be uncommented as the properties are "required".
Otherwise change them to be optional and leave them commented out.
If node is started with instrumentation and endpoint/attributes not provided, exist with error?

Actually, the otlp properties are not required in the config schema. You can see here. Also, the distributor-1 service works fine if I remove the JOYSTREAM_DISTRIBUTOR__OTLP__ENDPOINT, JOYSTREAM_DISTRIBUTOR__OTLP__ATTRIBUTES env vars from the service in docker-compose.yml file, which also enforces the point that otlp properties are not required. I think there is a problem with the ValidationService class. I tried to debug the problem by adding console.log(this.avj.errors) at line, and got this:

[ { keyword: 'required', dataPath: '/otlp', schemaPath: '#/properties/otlp/required', params: { missingProperty: 'attributes' }, message: "should have required property 'attributes'" } ] /joystream/distributor-node/lib/services/parsers/ConfigParserService.js:97 throw e; ^ ValidationError: Invalid env value of JOYSTREAM_DISTRIBUTOR__OTLP__ENDPOINT

Just created an issue for this. Will be investigating it #4787

okay, so how do we interpret the line required: ['endpoint', 'attributes'], in configSchema.ts

okay, so how do we interpret the line required: ['endpoint', 'attributes'], in configSchema.ts

This means otlp is an optional object in configSchema. However, when it is present (i.e., not undefined), it must have endpoint & attributes properties.

FYI I created a PR to address this issue, please have a look #4788

Merged the fix, perhaps you want to update this from master

…ry_instrumentation

mnaamani

Looks and works as expected.
Just left some comments about possible generalization and decoupling the instrumentation config from the main application.

mnaamani · 2023-06-13T07:59:03Z

distributor-node/src/app/instrumentation.ts

+
+diag.info('Starting tracing...')
+
+// Default config JSON/YAML file path (relative to current working directory)


I'm glad we discovered issue with parsing the config because of adding the new parameters in the config file, however I feel perhaps we can just rely on OTEL_... env variables, since they are not really configuration parameters for the distributor-node itself. And by dropping this coupling, where we are importing from the

import { ConfigParserService } from '../services/parsers/ConfigParserService' import { ReadonlyConfig } from '../types'

It becomes possible to break this code out of the distributor package into its own package that can be re-used?

What are your thoughts. I'm not familiar enough with Open Telemetry architecture yet to have a very strong opinion on this.

kdembler · 2023-06-16T11:25:38Z

Okay it's been a while, really sorry for the delay. I'm starting testing it out right now

kdembler · 2023-06-16T14:13:10Z

distributor-node/CHANGELOG.md

@@ -1,5 +1,6 @@
 ### 1.2.0

+- Integrates OpenTelemetry API/SDK with Argus for exporting improved tracing logs & metrics to Elasticsearch. Adds `./start-elasticsearch-stack.sh` script to bootstrap elasticsearch services (Elasticsearch + Kibana + APM Server) with all the required configurations. 


Since 1.2.0 has been already released and is in use, let's bump the version please

kdembler · 2023-06-16T14:13:38Z

Also seeing ENOENT: no such file or directory, open '/joystream/distributor-node/src/commands/package.json'

zeeshanakram3 · 2023-07-10T12:41:17Z

It becomes possible to break this code out of the distributor package into its own package that can be re-used?

I have created @joystream/opentelemetry package in the monorepo that can be used to add opentelemetry tracing capabilities to any arbitrary nodes & services
Now all the OpenTelemetry code is Orthogonal to the Application packages and can be imported as a dependency
I tested the storage-node, distributed-node & query-node docker containers with open telemetry integration using the colossos-1, distributor-1 & graphql-server services respectively in the docker-compose.yml, and it seemed to work as expected
Building & running docker images of Applications (with OTEL integration) requires publishing of @joystream/opentelemetry on npm (so could not test that)

cc. @mnaamani

…com/zeeshanakram3/joystream into origo_opentelemetry_instrumentation

zeeshanakram3 · 2023-07-12T07:06:34Z

@mnaamani I have updated the dockerfiles to install @josyteam/opentelemtry in images during build step in d3a473f

mnaamani

Nice work. Left a few suggested changes, otherwise it works great and seeing nice traces in APM down to the SQL queries.

colossus.Dockerfile

distributor-node.Dockerfile

distributor-node/CHANGELOG.md

mnaamani · 2023-07-12T15:02:46Z

query-node/build.sh

+
+################################################
+# temporary patche TODO: create proper solution
+


Some env variables i think might be getting lost, there is a specific one i noticed which is set at top .env PROCESSOR_HOST=processor which is the host/ip the graphql server expects from the processor when it "pings" it to update. So the graphql-server is constantly logging:

2023-07-12 19:30:30 [ 2023-07-12 19:30:30 'Unauthorized access on /update-processor-state: 172.18.0.10 (expected: undefined)' 2023-07-12 19:30:30 ]

Ok, so it took a while to figure this out.

The PROCESSOR_HOST env var is available inside the graphql-server, how the problem is happening while dns lookup, if the graphql-server is running with open telemetry instrumentation enabled, then the lookup function returns an address string (response type of callback-based function) instead of expected LookupAddress object (response type of promise-based function). So, in short, open telemetry isn't applying a proper patch of the lookup function.

There are two possible solutions I can think of

Don't instrument the dns package in the graphql-server (a much simpler solution)

Make a change in the graphql-server codebase to use callback based or dns/promise based lookup method instead of promisify lookup function

Don't instrument the dns package in the graphql-server (a much simpler solution)

I think that is the best option to go with. 👍

Addressed in 7c658a8

start-elasticsearch-stack.sh

opentelemetry/index.ts

…script

…entation

mnaamani · 2023-07-13T18:37:33Z

opentelemetry/instrumentations/default.ts

+    // Disable DNS instrumentation, because the instrumentation does not correctly patches `dns.lookup` function
+    // if the function is converted to a promise-based method using `utils.promisify(dns.lookup)`
+    // See: https://github.com/Joystream/joystream/pull/4779#discussion_r1262515887
+    getNodeAutoInstrumentations({ '@opentelemetry/instrumentation-dns': { enabled: false } }),


This also fixed a problem I noticed with storage and distributor nodes, not finishing reponse on their status endpoint /api/v1/status

mnaamani

A few minor fixes, I prepared them in a PR zeeshanakram3#6

zeeshanakram3 added 2 commits June 1, 2023 23:27

remove outdated filebeat config & Dockerfile

02e036b

add @opentelemetry/ dependencies & instrumentation

cca088b

zeeshanakram3 added 4 commits June 2, 2023 08:26

added otlp.endpoint & otlp.attributes configuration options

b3c9a43

add docker-compose.elasticsearch.yml & start-elasticsearch-stack.sh

257e492

update distributor-node/docker-compose.yml

967060a

fix lint

8c0494e

zeeshanakram3 marked this pull request as ready for review June 2, 2023 04:41

zeeshanakram3 requested a review from mnaamani June 2, 2023 04:42

zeeshanakram3 mentioned this pull request Jun 2, 2023

[Argus] better request tracing logs #4763

Closed

mnaamani requested changes Jun 8, 2023

View reviewed changes

zeeshanakram3 mentioned this pull request Jun 8, 2023

Argus: ConfigParserService expects value to be provided even for optional property #4787

Closed

mnaamani self-requested a review June 12, 2023 18:05

zeeshanakram3 added 3 commits June 13, 2023 01:10

Merge remote-tracking branch 'upstream/master' into origo_opentelemet…

19301d4

…ry_instrumentation

generate config docs

55765b0

update Argus CHANGELOG

a0515b9

mnaamani requested a review from kdembler June 13, 2023 07:44

mnaamani reviewed Jun 13, 2023

View reviewed changes

kdembler reviewed Jun 16, 2023

View reviewed changes

zeeshanakram3 added the argus Argus distributor node label Jun 23, 2023

kdembler mentioned this pull request Jun 24, 2023

Argus: QN is sometimes very slow #4795

Closed

zeeshanakram3 added 6 commits July 10, 2023 16:24

remove otel configuration from distributor-node package

b619040

add generalized @joystream/opentelemetry package

b4855dc

add otel instrumentation for query node

4f5d034

add otel instrumentation for storage node

8d4eb8c

WIP: a0515b9 update Argus CHANGELOG

08ed70d

run docker services wit otel tracing

a4e7154

zeeshanakram3 added 2 commits July 11, 2023 01:50

fix: loading otel env vars

c964667

fix: query node build script

fabd8bb

zeeshanakram3 requested a review from mnaamani July 10, 2023 21:45

zeeshanakram3 added 3 commits July 12, 2023 11:53

otel: remove span names from diretibutor/storage-node instrumentations

c5f2699

install @joystream/opentelemetry inside docker images

d3a473f

Merge branch 'origo_opentelemetry_instrumentation' of https://github.…

80d7169

…com/zeeshanakram3/joystream into origo_opentelemetry_instrumentation

mnaamani requested changes Jul 12, 2023

View reviewed changes

zeeshanakram3 added 3 commits July 13, 2023 15:53

print service token generation error in start-elasticsearch-stach.sh …

b9aa169

…script

address change requests

ad084e9

fix: disable dns instrumentation in the default opentelemetry instrum…

7c658a8

…entation

zeeshanakram3 mentioned this pull request Jul 13, 2023

Don't use utils.promisify(dns.lookup) as OpenTelemetey's DNS instrumentation can't patch such function properly Joystream/hydra#525

Open

mnaamani reviewed Jul 13, 2023

View reviewed changes

mnaamani self-requested a review July 13, 2023 18:47

mnaamani added 3 commits July 14, 2023 01:22

fix no bind volumes docker image tests

3f987d2

no docker login in pull_request flow

b33e3bd

docker-compose fix db name and running tests in apps workflow

3ec8eeb

mnaamani requested changes Jul 14, 2023

View reviewed changes

Run SP/DP/QN without telemetry in docker-compose setup

de501f8

mnaamani self-requested a review July 14, 2023 13:38

mnaamani approved these changes Jul 14, 2023

View reviewed changes

mnaamani merged commit 8a2a812 into Joystream:master Jul 14, 2023
23 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Origo: [Argus] Opentelemetry instrumentation for better metrics & tracing #4779

Origo: [Argus] Opentelemetry instrumentation for better metrics & tracing #4779

zeeshanakram3 commented Jun 1, 2023

vercel bot commented Jun 1, 2023 •

edited

Loading

zeeshanakram3 commented Jun 2, 2023

zeeshanakram3 commented Jun 2, 2023

kdembler commented Jun 2, 2023

mnaamani commented Jun 8, 2023 •

edited

Loading

mnaamani commented Jun 8, 2023 •

edited

Loading

mnaamani commented Jun 8, 2023

mnaamani commented Jun 8, 2023

mnaamani commented Jun 8, 2023

mnaamani Jun 8, 2023

zeeshanakram3 Jun 8, 2023

zeeshanakram3 Jun 8, 2023

mnaamani Jun 9, 2023

zeeshanakram3 Jun 9, 2023

mnaamani Jun 12, 2023

zeeshanakram3 Jun 13, 2023

mnaamani left a comment

mnaamani Jun 13, 2023

kdembler commented Jun 16, 2023 •

edited

Loading

kdembler Jun 16, 2023

kdembler commented Jun 16, 2023

zeeshanakram3 commented Jul 10, 2023

zeeshanakram3 commented Jul 12, 2023

mnaamani left a comment

mnaamani Jul 12, 2023

zeeshanakram3 Jul 13, 2023

mnaamani Jul 13, 2023

zeeshanakram3 Jul 13, 2023

mnaamani Jul 13, 2023 •

edited

Loading

mnaamani left a comment


		diag.info('Starting tracing...')

		// Default config JSON/YAML file path (relative to current working directory)

		@@ -1,5 +1,6 @@
		### 1.2.0

		- Integrates OpenTelemetry API/SDK with Argus for exporting improved tracing logs & metrics to Elasticsearch. Adds `./start-elasticsearch-stack.sh` script to bootstrap elasticsearch services (Elasticsearch + Kibana + APM Server) with all the required configurations.


		################################################
		# temporary patche TODO: create proper solution

Origo: [Argus] Opentelemetry instrumentation for better metrics & tracing #4779

Origo: [Argus] Opentelemetry instrumentation for better metrics & tracing #4779

Conversation

zeeshanakram3 commented Jun 1, 2023

vercel bot commented Jun 1, 2023 • edited Loading

zeeshanakram3 commented Jun 2, 2023

How to Test

zeeshanakram3 commented Jun 2, 2023

kdembler commented Jun 2, 2023

mnaamani commented Jun 8, 2023 • edited Loading

mnaamani commented Jun 8, 2023 • edited Loading

mnaamani commented Jun 8, 2023

mnaamani commented Jun 8, 2023

mnaamani commented Jun 8, 2023

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnaamani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

kdembler commented Jun 16, 2023 • edited Loading

Choose a reason for hiding this comment

kdembler commented Jun 16, 2023

zeeshanakram3 commented Jul 10, 2023

zeeshanakram3 commented Jul 12, 2023

mnaamani left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

mnaamani Jul 13, 2023 • edited Loading

Choose a reason for hiding this comment

mnaamani left a comment

Choose a reason for hiding this comment

vercel bot commented Jun 1, 2023 •

edited

Loading

mnaamani commented Jun 8, 2023 •

edited

Loading

mnaamani commented Jun 8, 2023 •

edited

Loading

kdembler commented Jun 16, 2023 •

edited

Loading

mnaamani Jul 13, 2023 •

edited

Loading