Skip to content
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

Align Host and Worker sample rate in Azure Functions module AI tracing #252

Open
wants to merge 10 commits into
base: main
Choose a base branch
from

Conversation

gquadrati
Copy link

@gquadrati gquadrati commented Jan 30, 2025

This pull request aims to align the sample rate settings for both the Host and Worker in the Azure Functions module's AI tracing. The changes ensure consistency and potentially improve the reliability of Application Insights sampling.

Copy link

changeset-bot bot commented Jan 30, 2025

🦋 Changeset detected

Latest commit: 7bcd11c

The changes in this PR will be included in the next version bump.

This PR includes changesets to release 2 packages
Name Type
azure_function_app Patch
azure_function_app_exposed Patch

Not sure what this means? Click here to learn what changesets are.

Click here if you're a maintainer who wants to add another changeset to this PR

gunzip

This comment was marked as outdated.

@Krusty93
Copy link
Contributor

N.B. If the changes will be approved, they should be backported in these other modules:

  • function app exposed
  • app service (?)
  • app service exposed (?)

@gunzip
Copy link
Contributor

gunzip commented Jan 30, 2025

Only in function app exposed @Krusty93. These are settings that are valid only for Azure Functions not App Services.

@gunzip gunzip changed the title Set fixed-rate logging strategy to AI function runtime, reduce log of type traces Align Host and Worker sample rate in Azure Functions module AI tracing Feb 2, 2025
@gunzip gunzip marked this pull request as ready for review February 28, 2025 09:37
@gunzip gunzip requested a review from a team as a code owner February 28, 2025 09:37
@gunzip
Copy link
Contributor

gunzip commented Feb 28, 2025

I'm unsure about the 'patch' changeset since this wil require a restart once applied. Maybe it would be better to use a major release?

@kin0992
Copy link
Contributor

kin0992 commented Feb 28, 2025

I'm unsure about the 'patch' changeset since this wil require a restart once applied. Maybe it would be better to use a major release?

@gunzip In my opinion, this qualifies as a patch since the scope is limited to the module, and the change does not impact any running resources.

Are you assuming that the reference to this Terraform module follows the pattern ~> x? If so, it's the responsibility of the team using this module to ensure that the feature is properly propagated to the production environment.

From this module's perspective, it’s simply a patch to me.

@gunzip gunzip enabled auto-merge (squash) February 28, 2025 16:00
@gunzip gunzip disabled auto-merge February 28, 2025 16:00
Copy link
Contributor

@kin0992 kin0992 left a comment

Choose a reason for hiding this comment

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

Well done!

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.

5 participants