Skip to content

feat: add resource name for Haystack Component Datadog spans#9337

Merged
sjrl merged 4 commits intodeepset-ai:mainfrom
lan666as:feat/datadog-tracer/set-component-name-as-resource-name
Jun 18, 2025
Merged

feat: add resource name for Haystack Component Datadog spans#9337
sjrl merged 4 commits intodeepset-ai:mainfrom
lan666as:feat/datadog-tracer/set-component-name-as-resource-name

Conversation

@lan666as
Copy link
Copy Markdown
Contributor

@lan666as lan666as commented May 2, 2025

Related Issues

Proposed Changes:

Set Datadog span resource name as component name and type instead of the operation name (i.e., haystack.component.run)

How did you test it?

  • Unit test
  • Manual verification at Datadog APM

Before

image

After

image

Notes for the reviewer

-

Checklist

  • I have read the contributors guidelines and the code of conduct
  • I have updated the related issue with new insights and changes
  • I added unit tests and updated the docstrings
  • I've used one of the conventional commit types for my PR title: fix:, feat:, build:, chore:, ci:, docs:, style:, refactor:, perf:, test: and added ! in case the PR includes breaking changes.
  • I documented my code
  • I ran pre-commit hooks and fixed any issue

@CLAassistant
Copy link
Copy Markdown

CLAassistant commented May 2, 2025

CLA assistant check
All committers have signed the CLA.

@github-actions github-actions Bot added the type:documentation Improvements on the docs label May 2, 2025
@sjrl
Copy link
Copy Markdown
Contributor

sjrl commented May 5, 2025

@lan666as thanks for the work on this! We will be able to give this a review once you are able to add tests and mark the PR as ready.

@coveralls
Copy link
Copy Markdown
Collaborator

coveralls commented May 5, 2025

Pull Request Test Coverage Report for Build 15728163096

Warning: This coverage report may be inaccurate.

This pull request's base commit is no longer the HEAD commit of its target branch. This means it includes changes from outside the original pull request, including, potentially, unrelated coverage changes.

Details

  • 0 of 0 changed or added relevant lines in 0 files are covered.
  • 2 unchanged lines in 1 file lost coverage.
  • Overall coverage increased (+0.009%) to 90.154%

Files with Coverage Reduction New Missed Lines %
tracing/datadog.py 2 96.0%
Totals Coverage Status
Change from base Build 15727603018: 0.009%
Covered Lines: 11564
Relevant Lines: 12827

💛 - Coveralls

@lan666as
Copy link
Copy Markdown
Contributor Author

Hi @sjrl, it seems the changes needed for our requirements are more extensive than anticipated. I've decided to fork the class to implement these independently and will be closing this PR. Thanks!

@lan666as lan666as closed this May 16, 2025
@lan666as lan666as deleted the feat/datadog-tracer/set-component-name-as-resource-name branch May 16, 2025 08:48
@lan666as lan666as restored the feat/datadog-tracer/set-component-name-as-resource-name branch June 18, 2025 02:41
@lan666as lan666as reopened this Jun 18, 2025
Comment thread haystack/tracing/datadog.py Outdated
@sjrl
Copy link
Copy Markdown
Contributor

sjrl commented Jun 18, 2025

@lan666as looking good! Could you also update tests in haystack/tracing/datadog.py to unit test this as well? I think just updating test_opentelemetry_tracer inside of the class TestDatadogTracer would be enough to test that resource name is also set in the trace.

@lan666as lan666as force-pushed the feat/datadog-tracer/set-component-name-as-resource-name branch from c319db5 to bba3f8e Compare June 18, 2025 06:14
@lan666as lan666as marked this pull request as ready for review June 18, 2025 06:17
@lan666as lan666as requested a review from a team as a code owner June 18, 2025 06:17
@lan666as lan666as requested review from anakin87 and removed request for a team June 18, 2025 06:17
@sjrl sjrl removed the request for review from anakin87 June 18, 2025 06:35
@lan666as
Copy link
Copy Markdown
Contributor Author

Let me resolve the CI issue first

@sjrl
Copy link
Copy Markdown
Contributor

sjrl commented Jun 18, 2025

@lan666as don't worry about the formatting CI that will be resolved with this PR #9528

But if you could add a release note that would be great! Instructions for adding a release note can be found here

@lan666as lan666as requested a review from a team as a code owner June 18, 2025 08:43
@lan666as lan666as requested review from dfokina and removed request for a team June 18, 2025 08:43
Signed-off-by: Ahmad Zidan <ahmad.zidan@traveloka.com>
@lan666as lan666as force-pushed the feat/datadog-tracer/set-component-name-as-resource-name branch from 57c322d to 987f98a Compare June 18, 2025 08:44
Copy link
Copy Markdown
Contributor

@sjrl sjrl left a comment

Choose a reason for hiding this comment

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

Looks good!

@sjrl sjrl enabled auto-merge (squash) June 18, 2025 09:10
@sjrl sjrl merged commit f911459 into deepset-ai:main Jun 18, 2025
19 checks passed
@lan666as lan666as deleted the feat/datadog-tracer/set-component-name-as-resource-name branch June 18, 2025 09:33
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

topic:tests type:documentation Improvements on the docs

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Datadog Tracer: Set Resource Name from Component Type/Name

4 participants