prime-simplereport icon indicating copy to clipboard operation
prime-simplereport copied to clipboard

Bump applicationinsights from 2.9.5 to 3.0.1 in /ops/services/app_functions/report_stream_batched_publisher/functions

Open dependabot[bot] opened this issue 1 year ago β€’ 7 comments

Bumps applicationinsights from 2.9.5 to 3.0.1.

Release notes

Sourced from applicationinsights's releases.

3.0.0

#1282 Fix Auto Exception Collection. #1292 Update Warning Tests and Add Console Logging by Default. #1296 Update agent backoff logic. #1298 Update Support Status of Native Metrics & Env Vars. #1300 Clean Up Shim Files & Fix Check for Extended Metrics Env Var. #1302 Add Shim Performance Testing. #1306 Update Unsupported Messages & Remove Deprecated SemAttrs.

3.0.0 beta 12

#1300 Clean Up Shim Files & Fix Check for Extended Metrics Env Var. #1298 Update Support Status of Native Metrics & Env Vars. #1296 Update agent backoff logic #1292 Update Warning Tests and Add Console Logging by Default. #1282 Fix Auto Exception Collection. #1275 Add Note Regarding Our Support for Node Versions. #1272 Auto collection of logs aligning to OpenTelemetry. #1271 Update Prefix Version in Auto-Attach Scenarios. #1270 Update Instrumentation Options Type in Docs and Export from Index. #1268 Detect Distro Already Loaded in Agent. #1266 Update Node.js runtime check in Agent. #1264 Update Beta Code Examples README.

3.0.0 beta 11

#1262 Extend the shim to support live metrics and the browser SDK Loader. #1256 Shift to using the diag API for logging. #1254 Update the shim support table to reflect changes in Azure Monitor OpenTelemetry. #1248 Fix serialization of logged errors. #1227 Remove duplicated types. #1228 Fix request performance counters not being calculated correctly. #1236 Fix Linux platform check in the agent.

3.0.0 beta 10

#1221 Increase Testing Code Coverage. #1223 Add env check to initialize OTLP Metrics in AKS. #1228 Remove duplicate types. #1228 Fix issue with performance counters not being calculated correctly.

3.0.0 beta.9

#1203 Fix JSON Configuration in the shim. #1206 Stringify objects when building a LogRecord #1208 Use @​azure/monitor-opentelemetry #1209 Rename Options interface to AzureMonitorOpenTelemetryOptions #1210 Update the README to include shim support information.

3.0.0 beta.8

#1201 Shadow Azure Monitor OpenTelemetry Distro #1197 Add OTLP Exporters and Perf Counters #1194 Consume latest Azure Monitor OpenTelemetry

... (truncated)

Commits

Dependabot compatibility score

Dependabot will resolve any conflicts with this PR as long as you don't alter it yourself. You can also trigger a rebase manually by commenting @dependabot rebase.


Dependabot commands and options

You can trigger Dependabot actions by commenting on this PR:

  • @dependabot rebase will rebase this PR
  • @dependabot recreate will recreate this PR, overwriting any edits that have been made to it
  • @dependabot merge will merge this PR after your CI passes on it
  • @dependabot squash and merge will squash and merge this PR after your CI passes on it
  • @dependabot cancel merge will cancel a previously requested merge and block automerging
  • @dependabot reopen will reopen this PR if it is closed
  • @dependabot close will close this PR and stop Dependabot recreating it. You can achieve the same result by closing it manually
  • @dependabot show <dependency name> ignore conditions will show all of the ignore conditions of the specified dependency
  • @dependabot ignore this major version will close this PR and stop Dependabot creating any more for this major version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this minor version will close this PR and stop Dependabot creating any more for this minor version (unless you reopen the PR or upgrade to it yourself)
  • @dependabot ignore this dependency will close this PR and stop Dependabot creating any more for this dependency (unless you reopen the PR or upgrade to it yourself)

dependabot[bot] avatar Apr 28 '24 15:04 dependabot[bot]

@dependabot rebase

emyl3 avatar May 07 '24 21:05 emyl3

Based on my digging around... Looks like v3 of applicationinsights uses Azure Monitor OpenTelemetry.

Our function tests are failing because tagOverrides does not seem to be supported in v3 vs v2.

I think this might be related to the Node.js version of OpenTelemetry not supporting certain overrides. My guess is that even though they don't explicitly call out Operation Id it might be included in what's not being supported πŸ˜… (see screenshot below)

Screenshot 2024-05-09 at 21 42 04

We are currently using tagOverrides property to override the "Operation Id" of our custom events. (It's the same value as the Parent Id that we also track) I'm not sure why we wanted to override this in the first place 🧐

If overriding this is important to us, then I don't think we should upgrade to this version.

Let me know what you all think... πŸ€”

emyl3 avatar May 10 '24 03:05 emyl3

Based on my digging around... Looks like v3 of applicationinsights uses Azure Monitor OpenTelemetry.

Our function tests are failing because tagOverrides does not seem to be supported in v3 vs v2.

I think this might be related to the Node.js version of OpenTelemetry not supporting certain overrides. My guess is that even though they don't explicitly call out Operation Id it might be included in what's not being supported πŸ˜… (see screenshot below)

Screenshot 2024-05-09 at 21 42 04 We are currently using [`tagOverrides` property to override the "Operation Id"](https://github.com/search?q=repo%3ACDCgov%2Fprime-simplereport+ai.operation.id&type=code) of our custom events. (It's the same value as the Parent Id that we also track) I'm not sure why we wanted to override this in the first place 🧐

If overriding this is important to us, then I don't think we should upgrade to this version.

Let me know what you all think... πŸ€”

I think if it's the case that we just pull that value from the parent ID, we should be fine without it. Guessing the overrides is helpful in tracing events through Azure because there's one point of reference for where the event got kicked off, but having the parent ID should be good enough. Johanna didn't seem to call it out explicitly in her PR putting it in, so I think we'd be good removing it and just using the parent ID.

Thanks for looking into it!

fzhao99 avatar May 10 '24 20:05 fzhao99

Based on my digging around... Looks like v3 of applicationinsights uses Azure Monitor OpenTelemetry. Our function tests are failing because tagOverrides does not seem to be supported in v3 vs v2. I think this might be related to the Node.js version of OpenTelemetry not supporting certain overrides. My guess is that even though they don't explicitly call out Operation Id it might be included in what's not being supported πŸ˜… (see screenshot below) Screenshot 2024-05-09 at 21 42 04 We are currently using tagOverrides property to override the "Operation Id" of our custom events. (It's the same value as the Parent Id that we also track) I'm not sure why we wanted to override this in the first place 🧐 If overriding this is important to us, then I don't think we should upgrade to this version. Let me know what you all think... πŸ€”

I think if it's the case that we just pull that value from the parent ID, we should be fine without it. Guessing the overrides is helpful in tracing events through Azure because there's one point of reference for where the event got kicked off, but having the parent ID should be good enough. Johanna didn't seem to call it out explicitly in her PR putting it in, so I think we'd be good removing it and just using the parent ID.

Thanks for looking into it!

Ok! Will update this and remove that override!

emyl3 avatar May 14 '24 19:05 emyl3

Deployed to dev2 and looking into this exception πŸ‘€

Exception while executing function: Functions.QueueBatchedReportStreamUploader Result: Failure
Exception: Cannot read properties of undefined (reading 'trackEvent')
Stack: TypeError: Cannot read properties of undefined (reading 'trackEvent')
    at TelemetryClient.trackEvent (/home/site/wwwroot/node_modules/applicationinsights/out/src/shim/telemetryClient.js:106:22)
    at /home/site/wwwroot/dist/QueueBatchedReportStreamUploader/index.js:53:19
    at Generator.next (<anonymous>)
    at fulfilled (/home/site/wwwroot/dist/QueueBatchedReportStreamUploader/index.js:28:58)
    at process.processTicksAndRejections (node:internal/process/task_queues:95:5) 

emyl3 avatar May 15 '24 15:05 emyl3

v3 of applicationinsights needs an environment variable called APPLICATIONINSIGHTS_CONNECTION_STRING (see README snippet below)

Configure the local SDK by calling appInsights.setup('YOUR_CONNECTION_STRING');, using the connection string you grabbed in step 2. Or put it in the APPLICATIONINSIGHTS_CONNECTION_STRING environment variable and call appInsights.setup() without parameters.

We currently have that environment variable set up in the api.tf file (dev2's for example) but we don't have that set for the function app because we were previously relying on APPINSIGHTS_INSTRUMENTATIONKEY but that is no longer supported Screenshot 2024-05-16 at 17 49 28

I think that should get our app insights to initialize correctly, but I will have to do more testing on dev2 after the env variable is introduced...

Do I just add APPLICATIONINSIGHTS_CONNECTION_STRING = data.azurerm_application_insights.app_insights.connection_string to ops/services/app_functions/report_stream_batched_publisher/infra/main.tf to add it as an ENV variable? πŸ€”

cc @alismx, @shanice-skylight

emyl3 avatar May 16 '24 22:05 emyl3

v3 of applicationinsights needs an environment variable called APPLICATIONINSIGHTS_CONNECTION_STRING (see README snippet below)

Configure the local SDK by calling appInsights.setup('YOUR_CONNECTION_STRING');, using the connection string you grabbed in step 2. Or put it in the APPLICATIONINSIGHTS_CONNECTION_STRING environment variable and call appInsights.setup() without parameters.

We currently have that environment variable set up in the api.tf file (dev2's for example) but we don't have that set for the function app because we were previously relying on APPINSIGHTS_INSTRUMENTATIONKEY but that is no longer supported Screenshot 2024-05-16 at 17 49 28

I think that should get our app insights to initialize correctly, but I will have to do more testing on dev2 after the env variable is introduced...

Do I just add APPLICATIONINSIGHTS_CONNECTION_STRING = data.azurerm_application_insights.app_insights.connection_string to ops/services/app_functions/report_stream_batched_publisher/infra/main.tf to add it as an ENV variable? πŸ€”

cc @alismx, @shanice-skylight

I think you're on the right path! Because of the name of the data object in the _data.tf file it would probably be APPLICATIONINSIGHTS_CONNECTION_STRING = data.azurerm_application_insights.app.connection_string

alismx avatar May 17 '24 17:05 alismx

v3 of applicationinsights needs an environment variable called APPLICATIONINSIGHTS_CONNECTION_STRING (see README snippet below)

Configure the local SDK by calling appInsights.setup('YOUR_CONNECTION_STRING');, using the connection string you grabbed in step 2. Or put it in the APPLICATIONINSIGHTS_CONNECTION_STRING environment variable and call appInsights.setup() without parameters.

We currently have that environment variable set up in the api.tf file (dev2's for example) but we don't have that set for the function app because we were previously relying on APPINSIGHTS_INSTRUMENTATIONKEY but that is no longer supported Screenshot 2024-05-16 at 17 49 28 I think that should get our app insights to initialize correctly, but I will have to do more testing on dev2 after the env variable is introduced... Do I just add APPLICATIONINSIGHTS_CONNECTION_STRING = data.azurerm_application_insights.app_insights.connection_string to ops/services/app_functions/report_stream_batched_publisher/infra/main.tf to add it as an ENV variable? πŸ€” cc @alismx, @shanice-skylight

I think you're on the right path! Because of the name of the data object in the _data.tf file it would probably be APPLICATIONINSIGHTS_CONNECTION_STRING = data.azurerm_application_insights.app.connection_string

Ohh thank you for the help! I'll try that out πŸ™‡

emyl3 avatar May 17 '24 18:05 emyl3

Yay! It worked Screenshot 2024-05-17 at 15 43 27 Deployed on dev2 Ready for review @fzhao99 @mpbrown @DanielSass

emyl3 avatar May 17 '24 20:05 emyl3

One thing I am noticing is that the event that is logged doesn't contain the ParentId as I had hoped... Screenshot 2024-05-17 at 15 48 00

emyl3 avatar May 17 '24 20:05 emyl3

I'll add it under properties

emyl3 avatar May 17 '24 20:05 emyl3

hmmm for some reason adding that causes 00-PARENTID-00 to be added πŸ€” See this in application insights

Edit: Actually this looks fine as it matches what was happening before πŸ˜…

emyl3 avatar May 17 '24 22:05 emyl3

A newer version of applicationinsights exists, but since this PR has been edited by someone other than Dependabot I haven't updated it. You'll get a PR for the updated version as normal once this PR is merged.

dependabot[bot] avatar May 19 '24 15:05 dependabot[bot]

Oh my what a doozy -- thanks so much for getting this done!

bobbywells52 avatar May 21 '24 15:05 bobbywells52