Skip to content

Conversation

pulak-opti
Copy link
Contributor

Summary

  • Add OpenTelemetry tracing

Ticket

@pulak-opti pulak-opti changed the title [FSSDK-9705] Add OpenTelemetry tracing [FSSDK-9705] feat: add OpenTelemetry tracing Oct 10, 2023
@pulak-opti pulak-opti marked this pull request as ready for review October 10, 2023 11:14
Copy link
Contributor

@mikechu-optimizely mikechu-optimizely left a comment

Choose a reason for hiding this comment

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

Mostly just questions 👇 but otherwise LGTM...though probably should also get @jaeopt 's review.

Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

Looks great! A few suggestions and clarifications.

@pulak-opti pulak-opti requested a review from jaeopt October 11, 2023 13:02
@pulak-opti pulak-opti requested a review from jaeopt October 12, 2023 17:51
Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

All looks good! A nit suggested.
I still see the "tracing" instead of "opentelemetry: tracing" in the config.yml.
Wondering about the plan.

@pulak-opti
Copy link
Contributor Author

All looks good! A nit suggested. I still see the "tracing" instead of "opentelemetry: tracing" in the config.yml. Wondering about the plan.

ah sorry, config changes are fall behind for distributed tracing. btw I am still thinking what the config should be.
now tracing > opentelemetry makes more sense if we have other tracing implementation in future.

@pulak-opti pulak-opti requested a review from jaeopt October 18, 2023 07:15
Copy link
Contributor

@jaeopt jaeopt left a comment

Choose a reason for hiding this comment

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

LGTM

@pulak-opti pulak-opti merged commit b2bdbd5 into master Oct 19, 2023
@pulak-opti pulak-opti deleted the pulak/otel-tracing branch October 19, 2023 16:04
pulak-opti added a commit that referenced this pull request Nov 2, 2023
* add a basic tracing pipeline

* add tracing config

* update tracing config

* use context

* update handler with middleware

* support both http & grpc protocal

* refactor config

* add unit test

* update unit test

* refactor code

* make tracing disabled by default

* add config doc

* fix typo

* collect more attributes

* add common http attributes

* collect http status code attribute

* collect sdk key attribute

* support distributed tracing

* add unit test for trace id generator

* update unit test

* fix typo

* fix typo in config file

* update config.yaml

* make trace_id configurable for distributed tracing
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants