Update telemetry for getDeltas from db

[shubhi1092]

Description

• Adding metric to capture getDeltas method.
• Adding "caller" property to the telemetry to identify internal vs external calls.

[zhangxin511]

The changes make sense for OSS, however, FRS is not using this piece of logic. Don't see the benefits the metrics added here.

FRS has a file called fallbackDeltaService.ts, which has almost all the metric/status you need. Maybe take a look over there?

[shubhi1092]

Hey Xin, fallbackDeltaService in FRS extends from OSS's deltaService and calls super.getDeltas to get the deltas from db. So GetDeltasFromDb will be useful for both FRS as well as OSS. GetDeltasFromStorage also seemed similar but I was not sure if its indeed used or not, but there's no harm in adding that as well.

[zhangxin511]

Oh I see, make sense. I still don't think you need to add a metric here, the reason is it will double the metric/log numbers for each getdelta call. Instead, you can add a property flag in fallbackDeltaService, and it can indicate if it pass the super.getDeltas successfuly. If not, the fallbackDeltaService will still log the error, or just use the stage flag to indicate. Doing so will keep the log low and keep it simple to analysis (you have everything in one metric so you don't need to do the join correlate different metrics, what you think?

I don't think GetDeltasFromStorage is still being used anywhere, you can find the usage which is by one route, and there is no such route logs in our kusto for past 30 days. So I think this endpoint is an obsolete endpoint

[shubhi1092]

I think adding this metric here would help us in analysing the time taken for the db call during the whole get deltas API duration. We can still do that outside in the fallbackDeltaService, but I thought this would be cleaner since we also have a similar metric for GetDeltasFromFallback to capture that duration.

If amount of logs is a concern, I can alternately add the duration for both GetDeltasFromDb as well as GetDeltasFromFallback to the overall GetDeltaObjectStoreFallback metric and remove this metric from OSS then - what do you think?

I will remove the GetDeltasFromStorage metric since that is obsolete and not needed.

[zhangxin511]

It is up to you, I prefer to use single metrics instead of double the metrics due to

1. The amount of logs, we currently already have too many logs crashing geneva buffer, which lead to lost logs. Also it leads to kusto ingestion delay. @hedasilv do you have any concerns? Given the GetDeltaObjectStoreFallback log is already number 5 on the list
2. it is difficult to correlate the two metric from the same call. But if you can figure out a better way to correlate, I think it is fine.

Execute in [Web] [Desktop] [cluster('frspme.westus2.kusto.windows.net').database('FRS')]

FRSRouterlicious

| where eventTimestamp > ago(1d)

| summarize count() by eventName

| order by count_ desc

| take 10

Table0

eventName	count_
LogMessage	1,417,995,212
RiddlerFetchTenantKey	926,002,068
RunWithRetry	138,144,104
ScriptoriumProcessBatch	127,897,144
GetDeltaObjectStoreFallback	84,785,599
ReprocessOps	67,206,731
DisconnectDocument	63,701,361
ConnectDocument	58,862,813
ConnectDocumentGetClients	58,857,795
ConnectDocumentAddClient	52,860,490

[shubhi1092]

Hey @zhangxin511 , these are my thoughts and reasoning:

1. I see the benefit of adding a new metric here to analyse the GetDeltasFromDb trend, its faster to check and plot the direct duration of a metric instead of extracting that from the properties of another metric.
2. However I also see the benefit of adding this latency to the overall GetDeltaObjectStoreFallback's metric properties since its easier to check/correlate for a single call.

I am going to do # 2 anyways in FRS since we already have that metric. And since number of logs is a concern here, lets not do # 1 for now and will reconsider if needed after adding # 2.

[zhangxin511]

It is up to you, I prefer to use single metrics instead of double the metrics due to

1. The amount of logs, we currently already have too many logs crashing geneva buffer, which lead to lost logs. Also it leads to kusto ingestion delay. @hedasilv do you have any concerns? Given the GetDeltaObjectStoreFallback log is already number 5 on the list
2. it is difficult to correlate the two metric from the same call. But if you can figure out a better way to correlate, I think it is fine.

Execute in [Web] [Desktop] [cluster('frspme.westus2.kusto.windows.net').database('FRS')]

FRSRouterlicious

| where eventTimestamp > ago(1d)

| summarize count() by eventName

| order by count_ desc

| take 10

Table0

eventName	count_
LogMessage	1,417,995,212
RiddlerFetchTenantKey	926,002,068
RunWithRetry	138,144,104
ScriptoriumProcessBatch	127,897,144
GetDeltaObjectStoreFallback	84,785,599
ReprocessOps	67,206,731
DisconnectDocument	63,701,361
ConnectDocument	58,862,813
ConnectDocumentGetClients	58,857,795
ConnectDocumentAddClient	52,860,490

[zhangxin511]

remember to pass this from fallbackservice