Skip to content

OTEL instrumentation Support#1218

Merged
ritwik-g merged 35 commits into
mainfrom
tracing-intrumentation
Apr 7, 2025
Merged

OTEL instrumentation Support#1218
ritwik-g merged 35 commits into
mainfrom
tracing-intrumentation

Conversation

@ritwik-g

@ritwik-g ritwik-g commented Mar 30, 2025

Copy link
Copy Markdown
Contributor

What

  • Dependencies required for otel no code instrumentation and docker file changes for the same
  • Backend and Core logger changes to add trace_id and span_id in the logs
  • Runner and tool changes to support tracing in tools (Each transaction in tool will be separate and not connected to the runner trace since we are creating pod using docker or k8s client)
  • https://opentelemetry.io/docs/zero-code/python/

Why

  • To enable tracing

How

  • Using zero code instrumentation.

Can this PR break any existing features. If yes, please list possible items. If no, please explain why. (PS: Admins do not merge the PR without this section filled)

  • Ideally it shouldn't but will need to test without OTEL to confirm.

Database Migrations

  • NA

Env Config

Relevant Docs

Related Issues or PRs

Dependencies Versions

Notes on Testing

  • Tested only with OTEL enabled. Needs test without OTEL to make sure no errors occur

Screenshots

Checklist

I have read and understood the Contribution Guidelines.

ritwik-g and others added 29 commits March 30, 2025 03:11
@ritwik-g ritwik-g marked this pull request as ready for review April 4, 2025 17:32
@ritwik-g ritwik-g changed the title Tracing intrumentation Tracing instrumentation Support Apr 4, 2025
@ritwik-g ritwik-g changed the title Tracing instrumentation Support OTEL instrumentation Support Apr 4, 2025
Comment thread tools/structure/src/main.py Outdated
Comment thread tools/structure/src/main.py Outdated
Signed-off-by: Ritwik G <100672805+ritwik-g@users.noreply.github.com>
Comment thread tools/structure/src/main.py Outdated
@github-actions

github-actions Bot commented Apr 5, 2025

Copy link
Copy Markdown
Contributor
filepath function $$\textcolor{#23d18b}{\tt{passed}}$$ SUBTOTAL
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_logs}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_cleanup\_skip}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_client\_init}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image\_exists}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_image}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_get\_container\_run\_config\_without\_mount}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{runner/src/unstract/runner/clients/test\_docker.py}}$$ $$\textcolor{#23d18b}{\tt{test\_run\_container}}$$ $$\textcolor{#23d18b}{\tt{1}}$$ $$\textcolor{#23d18b}{\tt{1}}$$
$$\textcolor{#23d18b}{\tt{TOTAL}}$$ $$\textcolor{#23d18b}{\tt{9}}$$ $$\textcolor{#23d18b}{\tt{9}}$$

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

Copilot reviewed 13 out of 19 changed files in this pull request and generated 1 comment.

Files not reviewed (6)
  • docker/dockerfiles/backend.Dockerfile: Language not supported
  • docker/dockerfiles/platform.Dockerfile: Language not supported
  • docker/dockerfiles/prompt.Dockerfile: Language not supported
  • docker/dockerfiles/runner.Dockerfile: Language not supported
  • docker/dockerfiles/x2text.Dockerfile: Language not supported
  • tools/structure/Dockerfile: Language not supported

Comment thread runner/src/unstract/runner/runner.py
@sonarqubecloud

sonarqubecloud Bot commented Apr 5, 2025

Copy link
Copy Markdown

@chandrasekharan-zipstack chandrasekharan-zipstack left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM - @ritwik-g does OSS need these OTEL deps to be installed? Will OSS have tracing too?

@muhammad-ali-e muhammad-ali-e left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

LGTM overall structure

@ritwik-g ritwik-g merged commit 2bf6a4b into main Apr 7, 2025
@ritwik-g ritwik-g deleted the tracing-intrumentation branch April 7, 2025 06:41
pk-zipstack pushed a commit that referenced this pull request Aug 20, 2025
* Added OTEL instrumentation dependencies for tracing

* Moved gunicorn to main dependency and removed the version for otel instrumentations

* Commit pdm.lock changes

* Changed the deploy group from dev to optional

* Commit pdm.lock changes

* Corrected optional dependencies

* Commit pdm.lock changes

* Added otel config in all services. Added additional env logic in runner to support otel config passing

* Commit pdm.lock changes

* Corrected logs and runner

* corrected the wrong method name

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* reverted adding the otel ids since it causes errro when isntrumentation not present

* fixes

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* Added missing dependency for tracing

* Commit pdm.lock changes

* Trace context support for tool. Added logs with trace id in flask logging helper

* Fix line length

* [pre-commit.ci] auto fixes from pre-commit.com hooks

for more information, see https://pre-commit.ci

* fixed line length

* Commit pdm.lock changes

* Added version for otel sdk and apis

* Apply suggestions from code review

Signed-off-by: Ritwik G <100672805+ritwik-g@users.noreply.github.com>

* Update tools/structure/src/main.py

Signed-off-by: Ritwik G <100672805+ritwik-g@users.noreply.github.com>

* Removed instrumentation dependencies

* Commit pdm.lock changes

* Commit pdm.lock changes

---------

Signed-off-by: Ritwik G <100672805+ritwik-g@users.noreply.github.com>
Co-authored-by: pre-commit-ci[bot] <66853113+pre-commit-ci[bot]@users.noreply.github.com>
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.

4 participants