Skip to content

[MLOB] revert forced agentless and api key fetching #585

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Apr 16, 2025

Conversation

sabrenner
Copy link
Collaborator

@sabrenner sabrenner commented Apr 16, 2025

What does this PR do?

Reverts forced agentless_enabled=True and api_key fetching as part of the LLMObs init process for the lambda.

Motivation

We recently made a change to add our endpoints to the extension and add a lambda instrument command to use the latest layer versions and set

  • DD_LLMOBS_ENABLED=true
  • DD_LLMOBS_AGENTLESS_ENABLED=false to specifically use the agent in the extension
  • DD_LLMOBS_ML_APP to whatever was passed into the --llmobs command

By using the agent in the extension, we can take advantage of the API secret key parsing there, and reduce the overhead here for importing anything boto3-related. Additionally, it's now a more seamless experience overall.

Testing Guidelines

Verified with a build of the layer with the already-released extension layer.

Additional Notes

Types of Changes

  • Bug fix
  • New feature
  • Breaking change
  • Misc (docs, refactoring, dependency upgrade, etc.)

Check all that apply

  • This PR's description is comprehensive
  • This PR contains breaking changes that are documented in the description
  • This PR introduces new APIs or parameters that are documented and unlikely to change in the foreseeable future
  • This PR impacts documentation, and it has been updated (or a ticket has been logged)
  • This PR's changes are covered by the automated tests
  • This PR collects user input/sensitive content into Datadog
  • This PR passes the integration tests (ask a Datadog member to run the tests)

@sabrenner sabrenner marked this pull request as ready for review April 16, 2025 15:07
@sabrenner sabrenner requested a review from a team as a code owner April 16, 2025 15:07
agentless_enabled=True,
api_key=llmobs_api_key,
)
LLMObs.enable()
Copy link
Contributor

Choose a reason for hiding this comment

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

Would it be worth it to consolidate these two if llmobs_env_var blocks into a single location?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

i think we need to have them separate because we use LLMObs in two different scopes. if we imported and enabled in the same block, then we wouldn't be able to use it in the flush block without importing it again. i think it's OK to leave as-is for now

Copy link
Contributor

Choose a reason for hiding this comment

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

ahhh, great point! you're totally right.

@sabrenner sabrenner merged commit 8207aa2 into main Apr 16, 2025
60 checks passed
@sabrenner sabrenner deleted the sabrenner/revert-llmobs-force-agentless branch April 16, 2025 17:59
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.

2 participants