Skip to content

[AZINTS-3895] Trigger LFO script from quickstart#12

Merged
dd-mergequeue[bot] merged 6 commits into
mainfrom
benjamin.johnsonstaub/azints-3895/run-lfo-script
Sep 25, 2025
Merged

[AZINTS-3895] Trigger LFO script from quickstart#12
dd-mergequeue[bot] merged 6 commits into
mainfrom
benjamin.johnsonstaub/azints-3895/run-lfo-script

Conversation

@benjjs
Copy link
Copy Markdown
Collaborator

@benjjs benjjs commented Sep 24, 2025

Summary

Add a step to the quickstart script to create or update the log forwarder. Since we already have a log_forwarders step to collect the existing log forwarders, I've named this one upsert_log_forwarder. We perform this step only if we get a non-empty log forwarding config as part of the user selections.

Since the LFO script wants scopes in terms of subscriptions only, we flatten management groups into subscriptions and dedup before passing them as part of the log forwarder config.

In order to invoke the script from this script, I've separated out the business logic from the command line arg parsing in the main function.

Testing

I made some temporary modifications for testing purposes:

  • Pass only one LFO so that we are allowed to toggle log forwarding
  • Don't actually invoke the script since it isn't quite ready for our use case
  • Skip the steps to create an app registration
  • Add some prints

Then connected the script and submitted with log forwarding enabled:
Screenshot 2025-09-24 at 12 21 22 PM
Note that the log forwarder settings came through and the loading text is correct.

Now try submitting with log forwarding disabled:
Screenshot 2025-09-24 at 1 04 58 PM
Note that we do not enter the step.

@benjjs benjjs requested a review from a team as a code owner September 24, 2025 17:29
@datadog-official
Copy link
Copy Markdown

datadog-official Bot commented Sep 24, 2025

✅ Code Quality    ✅ Code Vulnerabilities    ✅ Library Vulnerabilities

🎉 All green!

🛠️ No new code quality issues
🛡️ No new code vulnerabilities
📚 No new vulnerable libraries detected

This comment will be updated automatically if new data arrives.
🔗 Commit SHA: 0a58d8a | Docs | Was this helpful? Give us feedback!

Comment thread azure/integration_quickstart/src/azure_integration_quickstart/setup.py Outdated
Copy link
Copy Markdown
Collaborator

@gpalmz gpalmz left a comment

Choose a reason for hiding this comment

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

Nit and question but LGTM

Comment thread azure/integration_quickstart/src/azure_integration_quickstart/setup.py Outdated
Comment thread azure/integration_quickstart/src/azure_integration_quickstart/setup.py Outdated
@benjjs benjjs force-pushed the benjamin.johnsonstaub/azints-3895/run-lfo-script branch from 0090886 to bd30404 Compare September 24, 2025 17:48
@benjjs benjjs force-pushed the benjamin.johnsonstaub/azints-3895/run-lfo-script branch from bd30404 to a3bc0fe Compare September 24, 2025 18:00

def upsert_log_forwarder(config: dict, subscriptions: set[Subscription]):
log_forwarder_config = Configuration(
management_group_id="", #TODO(AZINTS-3935): Make this not required
Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

PR for this is open: #13

Comment thread azure/integration_quickstart/src/azure_integration_quickstart/setup.py Outdated
Copy link
Copy Markdown
Member

@agulen agulen left a comment

Choose a reason for hiding this comment

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

lgtm, left a few comments. One thing to keep in mind moving forward is that setup.py is getting big. Now that we have zipapp working, consider breaking up or adding new logic into new files.

@benjjs
Copy link
Copy Markdown
Collaborator Author

benjjs commented Sep 25, 2025

lgtm, left a few comments. One thing to keep in mind moving forward is that setup.py is getting big. Now that we have zipapp working, consider breaking up or adding new logic into new files.

Yeah once I'm done with the launch blockers that is definitely something I want to do.

@benjjs
Copy link
Copy Markdown
Collaborator Author

benjjs commented Sep 25, 2025

/merge

@dd-devflow-routing-codex
Copy link
Copy Markdown

dd-devflow-routing-codex Bot commented Sep 25, 2025

View all feedbacks in Devflow UI.

2025-09-25 15:13:34 UTC ℹ️ Start processing command /merge


2025-09-25 15:13:40 UTC ℹ️ MergeQueue: pull request added to the queue

The expected merge time in main is approximately 1m (p90).


2025-09-25 15:14:57 UTC ℹ️ MergeQueue: This merge request was merged

@dd-mergequeue dd-mergequeue Bot merged commit 9cd3d5f into main Sep 25, 2025
3 checks passed
@dd-mergequeue dd-mergequeue Bot deleted the benjamin.johnsonstaub/azints-3895/run-lfo-script branch September 25, 2025 15:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants