opentelemetry-demo icon indicating copy to clipboard operation
opentelemetry-demo copied to clipboard

[paymentservice] Adds otel logging support

Open Kimbohlovette opened this issue 3 months ago • 20 comments

This PR adds otel instrumentation of the pino library to the payment service

Kimbohlovette avatar Mar 24 '24 19:03 Kimbohlovette

Okay, similar thing as with the accounting service. Any recommendations on what to do? @puckpuck

Kimbohlovette avatar Mar 25 '24 12:03 Kimbohlovette

@puckpuck I've just reformatted the require statements as you requested. Could you take a look?

Kimbohlovette avatar Mar 25 '24 13:03 Kimbohlovette

@puckpuck @austinlparker @julianocosta89 is there a library or a stable exporter for javascript that can be used to export logs to opentelemetry?

Kimbohlovette avatar Apr 01 '24 09:04 Kimbohlovette

I spoke with @Kimbohlovette today and asked him to reach out to the otel-js team and check if the OTel logs are actually usable. Whenever he gets a reply, he will either adjust this PR to properly export the logs, or close it as not implemented yet.

julianocosta89 avatar Apr 04 '24 10:04 julianocosta89

I answered on Slack as well, but for the record here: the OTel JS package @opentelemetry/instrumentation-pino does not yet support the logs bridge API for sending log records via OTLP. I intend to work on that, but haven't gotten to it yet. (FWIW, the other instrumentation JS logging framework instrumentations -- Bunyan and Winston -- have added logs bridge support recently.)

trentm avatar Apr 04 '24 17:04 trentm

@julianocosta89 I've done discussed with trentm I will use bunyan to implement this. So work is in progress

Kimbohlovette avatar Apr 04 '24 17:04 Kimbohlovette

@trentm @julianocosta89 I have replaced pino with bunyan and done the neccessary instrumentation. I would like you to test and check if logs and coming to otel correctly. And I would also like to ask if checking these logs in opentelemetry is something that I can do locally before I push

Kimbohlovette avatar Apr 05 '24 14:04 Kimbohlovette

@Kimbohlovette you can build this service locally and then start the demo locally. Once everything is up and running, you can navigate to grafana: localhost:8080/grafana and check if the logs are arriving in the demo chart.

julianocosta89 avatar Apr 08 '24 09:04 julianocosta89

@Kimbohlovette you can build this service locally and then start the demo locally. Once everything is up and running, you can navigate to grafana: localhost:8080/grafana and check if the logs are arriving in the demo chart.

Okay I am on it

Kimbohlovette avatar Apr 09 '24 12:04 Kimbohlovette

@julianocosta89, I've updated the environment variables. I would like you to look at the instrumentation. Aside from that I looked at the log api which is currently in experimental and it looks like something that can work fine if we we want to keep things simple

Kimbohlovette avatar Apr 10 '24 12:04 Kimbohlovette

@julianocosta89, I've updated the environment variables. I would like you to look at the instrumentation. Aside from that I looked at the log api which is currently in experimental and it looks like something that can work fine if we we want to keep things simple

We would like to have OTLP logs, if it works we will definitely get it merged.

julianocosta89 avatar Apr 10 '24 12:04 julianocosta89

@julianocosta89, I've updated the environment variables. I would like you to look at the instrumentation. Aside from that I looked at the log api which is currently in experimental and it looks like something that can work fine if we we want to keep things simple

We would like to have OTLP logs, if it works we will definitely get it merged.

Okay

Kimbohlovette avatar Apr 10 '24 13:04 Kimbohlovette

I've made the requested changes

Kimbohlovette avatar Apr 10 '24 13:04 Kimbohlovette

Please update your IDE settings to not modify the indentation and line wrap of code files. This will make reviewing the PR much easier.

I ran this branch, and did not see the traces or logs being emitted from the payment service. With a quicker look I noticed you added console exporters for both. I don't think you need any exporter, since the default should be OTLP, which is configured using environment variables.

puckpuck avatar Apr 11 '24 02:04 puckpuck

Please update your IDE settings to not modify the indentation and line wrap of code files. This will make reviewing the PR much easier.

I ran this branch, and did not see the traces or logs being emitted from the payment service. With a quicker look I noticed you added console exporters for both. I don't think you need any exporter, since the default should be OTLP, which is configured using environment variables.

Alright, I've been finding where in vscode I can disable the automatic formatting on vscode for some time now

Kimbohlovette avatar Apr 11 '24 07:04 Kimbohlovette

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions[bot] avatar Apr 19 '24 03:04 github-actions[bot]

Hello @pichlermarc, I have tried to use @opentelemetry/sdk-logs in this service to send logs to open telemetry, Unfortunately I can not see any logs emitted on grafana. Please can you look at my code to see what is not right?

Kimbohlovette avatar Apr 21 '24 17:04 Kimbohlovette

@trentm I tried bunyan but no lock too

Kimbohlovette avatar Apr 21 '24 17:04 Kimbohlovette

Hello @pichlermarc, I have tried to use @opentelemetry/sdk-logs in this service to send logs to open telemetry, Unfortunately I can not see any logs emitted on grafana. Please can you look at my code to see what is not right?

Looking at this PR's diff, it looks like there's no exporter configured other than the console log exporter. Are you using a different set of changes to export to grafana/loki? :thinking:

pichlermarc avatar Apr 22 '24 13:04 pichlermarc

Hello @pichlermarc, I have tried to use @opentelemetry/sdk-logs in this service to send logs to open telemetry, Unfortunately I can not see any logs emitted on grafana. Please can you look at my code to see what is not right?

Looking at this PR's diff, it looks like there's no exporter configured other than the console log exporter. Are you using a different set of changes to export to grafana/loki? 🤔

I don't have full understanding on how the logs gets to grafana dashboard but what I understand is that in this exercise open telemetry has already been configured to send logs to grafana (which is one of the servies in this app serving on 8080/grafana).

So the goal of this issue is to get the each service to send logs to open telemetry and then it will show on grafana under the current service

Kimbohlovette avatar Apr 22 '24 15:04 Kimbohlovette

This PR was marked stale due to lack of activity. It will be closed in 7 days.

github-actions[bot] avatar Apr 30 '24 03:04 github-actions[bot]

Closed as inactive. Feel free to reopen if this PR is still being worked on.

github-actions[bot] avatar May 07 '24 03:05 github-actions[bot]