Skip to content

Conversation

jviau
Copy link
Contributor

@jviau jviau commented Jul 30, 2024

Issue describing the changes in this PR

resolves #issue_for_this_pr

Pull request checklist

IMPORTANT: Currently, changes must be backported to the in-proc branch to be included in Core Tools and non-Flex deployments.

  • Backporting to the in-proc branch is not required
    • Otherwise: Link to backporting PR -- TODO
  • My changes do not require documentation changes
    • Otherwise: Documentation issue linked to PR
  • My changes should not be added to the release notes for the next release
    • Otherwise: I've added my notes to release_notes.md
  • My changes do not need to be backported to a previous version
    • Otherwise: Backport tracked by issue/PR #issue_or_pr
  • My changes do not require diagnostic events changes
    • Otherwise: I have added/updated all related diagnostic events and their documentation (Documentation issue linked to PR)
  • I have added all required tests (Unit tests, E2E tests)

Additional information

Our RPC function invoker is executed as part of WebJobs function invocation and thus defaults to being 'user' scope. This PR sets it back to system scope within that method so we will resume capturing these logs. This only affects our own logs, does not affect user Application Insights logs.

@jviau jviau requested a review from a team as a code owner July 30, 2024 21:07
Copy link
Member

@fabiocav fabiocav left a comment

Choose a reason for hiding this comment

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

Any tests we can put in place to validate this?

Copy link
Member

@fabiocav fabiocav left a comment

Choose a reason for hiding this comment

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

Approving, but if we can add some tests, particularly to ensure this isn't regressed, it would be great.

@jviau jviau merged commit c4b8e28 into dev Aug 1, 2024
@jviau jviau deleted the jviau/log/rpc-unsuppress branch August 1, 2024 20:09
jviau added a commit that referenced this pull request Aug 1, 2024
* Enable log system scope during RPC invocation

* Add unit test to verify scope

* Verify scope reverts to 'User'
kshyju pushed a commit that referenced this pull request Sep 4, 2024
* Enable log system scope during RPC invocation

* Add unit test to verify scope

* Verify scope reverts to 'User'
kshyju added a commit that referenced this pull request Sep 4, 2024
) (#10358)

* Enable log system scope during RPC invocation (#10344)

* Enable log system scope during RPC invocation

* Add unit test to verify scope

* Verify scope reverts to 'User'

* Remove usage of IHostMetrics type (in test) which does not exist in in-proc branch

---------

Co-authored-by: Shyju Krishnankutty <connectshyju@gmail.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.

3 participants