[RUMF-1176] collect other console logs new #1316

amortemousque · 2022-02-04T15:06:42Z

Motivation

Collect all console logs.
Under feature flag

Changes

Testing

Local
Staging
Unit
End to end

I have gone over the contributing documentation.

packages/core/src/browser/consoleObservable.ts

packages/core/src/tools/observable.ts

packages/logs/src/boot/startLogs.ts

packages/core/src/browser/consoleObservable.ts

packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts

packages/logs/src/boot/startLogs.ts

packages/core/src/browser/consoleObservable.ts

packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts

codecov-commenter · 2022-02-07T15:01:06Z

Codecov Report

Merging #1316 (109ce1f) into main (9b48504) will increase coverage by 0.06%.
The diff coverage is 92.94%.

@@            Coverage Diff             @@
##             main    #1316      +/-   ##
==========================================
+ Coverage   90.96%   91.03%   +0.06%     
==========================================
  Files         105      106       +1     
  Lines        4238     4281      +43     
  Branches      946      953       +7     
==========================================
+ Hits         3855     3897      +42     
- Misses        383      384       +1

Impacted Files	Coverage Δ
...omain/rumEventsCollection/error/errorCollection.ts	`76.00% <ø> (ø)`
...ain/rumEventsCollection/error/trackConsoleError.ts	`66.66% <66.66%> (ø)`
packages/logs/src/boot/startLogs.ts	`90.38% <80.95%> (+3.71%)`	⬆️
...kages/core/src/domain/console/consoleObservable.ts	`100.00% <100.00%> (ø)`
packages/core/src/tools/observable.ts	`100.00% <100.00%> (ø)`
packages/core/src/tools/utils.ts	`86.48% <100.00%> (+0.37%)`	⬆️
packages/logs/src/domain/configuration.ts	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9b48504...109ce1f. Read the comment docs.

packages/core/src/browser/consoleObservable.ts

BenoitZugmeyer

Appart from typechecking issue, LGTM!

packages/core/src/browser/consoleObservable.ts

bcaudan · 2022-02-08T17:01:16Z

packages/core/src/browser/consoleObservable.ts

+  handlingStack?: string
+}
+
+const consoleObservables: { [k in ConsoleApiName]?: Observable<ConsoleLog> } = {}


💬 suggestion: ‏Can you add a test for the need to have a singleton?

Isn't the "should allow multiple caller" test enough?

🤔 removing the singleton does not fail the test, so something is not right

It's because the observable references of consoleObservablesByApi live inside the merged observable.
I've updated the test to ensure the console api as been instrumented only once.

browser-sdk/packages/core/src/domain/console/consoleObservable.spec.ts

Line 65 in 18b2dbe

expect(instrumentedConsoleApi).toEqual(console[api])

packages/core/src/tools/utils.ts

packages/logs/src/domain/configuration.ts

packages/logs/src/boot/startLogs.ts

packages/core/src/browser/consoleObservable.ts

packages/logs/src/domain/configuration.ts

packages/core/src/tools/observable.spec.ts

packages/logs/src/boot/startLogs.ts

packages/logs/src/domain/configuration.ts

packages/logs/src/domain/configuration.spec.ts

amortemousque · 2022-02-11T14:58:59Z

packages/logs/src/boot/startLogs.spec.ts

@@ -83,6 +93,7 @@ describe('logs', () => {
    delete window.DD_RUM
    resetExperimentalFeatures()
    deleteEventBridgeStub()
+    stopSessionManager()


Remove test flackyness introduced with originalStartLogs in startLogs.spec.ts

packages/logs/src/domain/configuration.ts

amortemousque added 2 commits February 4, 2022 15:59

Add console observable

e552db1

🔥 Delete trackConsoleError

bf39e51

amortemousque requested a review from a team as a code owner February 4, 2022 15:06

amortemousque marked this pull request as draft February 4, 2022 15:07

amortemousque added 2 commits February 4, 2022 16:13

Add console observable to rum

4ba65d7

Add console observable to Logs

3ef80e6

amortemousque force-pushed the aymeric/collect-other-console-logs-new branch from cfe3837 to 3ef80e6 Compare February 4, 2022 15:14

BenoitZugmeyer reviewed Feb 4, 2022

View reviewed changes

bcaudan reviewed Feb 7, 2022

View reviewed changes

packages/logs/src/boot/startLogs.ts Outdated Show resolved Hide resolved

packages/core/src/browser/consoleObservable.ts Outdated Show resolved Hide resolved

packages/rum-core/src/domain/rumEventsCollection/error/errorCollection.ts Outdated Show resolved Hide resolved

👌 Add trackConsoleError

a1c5cbb

amortemousque added 2 commits February 7, 2022 16:57

👌 Update console log event observable

933269a

👌 Move source computation to Log and RUM

eed7e68

BenoitZugmeyer reviewed Feb 7, 2022

View reviewed changes

packages/core/src/browser/consoleObservable.ts Outdated Show resolved Hide resolved

amortemousque added 2 commits February 7, 2022 17:30

Use an enum + keep current behavior

e6a6b43

Add feature flag forward-logs

03a9de3

BenoitZugmeyer approved these changes Feb 7, 2022

View reviewed changes

amortemousque added 4 commits February 8, 2022 10:02

✅ Add tests

60ea273

✨ Add forwardConsoleLogs init config option

7ff6239

Fix rum console error tracking

1063bad

Merge branch 'main' into aymeric/collect-other-console-logs-new

cf3a4cd

amortemousque marked this pull request as ready for review February 8, 2022 15:57

✅ Add test for forwardErrorsToLogs: 'all'

7fc700c

bcaudan reviewed Feb 8, 2022

View reviewed changes

BenoitZugmeyer reviewed Feb 9, 2022

View reviewed changes

packages/core/src/browser/consoleObservable.ts Outdated Show resolved Hide resolved

packages/logs/src/domain/configuration.ts Show resolved Hide resolved

amortemousque added 5 commits February 9, 2022 12:13

👌 Namings

4abf4f2

👌 Ensure that only allowed api are instrumented.

9a874c3

👌 Remove CONSOLE_APIS

32a4393

👌 Clearer cast comment

4b8ea23

Update error observable naming

197dadc

amortemousque force-pushed the aymeric/collect-other-console-logs-new branch from 4fb75c9 to 197dadc Compare February 9, 2022 13:35

amortemousque added 5 commits February 9, 2022 14:36

👌 Move console observable into domain

d10b0dd

👌 Add mergeObservables tests

5cd4d91

✅ Update test to ensure the api is instrumented once

18b2dbe

Make removeDuplicates IE compatible

f88969c

Merge branch 'main' into aymeric/collect-other-console-logs-new

aa2669c

bcaudan reviewed Feb 10, 2022

View reviewed changes

bcaudan approved these changes Feb 10, 2022

View reviewed changes

amortemousque added 4 commits February 10, 2022 15:50

👌 Code style

d3792a5

👌 Add removeDuplicates test

3c3176c

Fix naming

fc3d57e

Fix test flackyness

7e26428

amortemousque commented Feb 11, 2022

View reviewed changes

Merge branch 'main' into aymeric/collect-other-console-logs-new

e35baea

BenoitZugmeyer reviewed Feb 15, 2022

View reviewed changes

packages/logs/src/domain/configuration.ts Outdated Show resolved Hide resolved

packages/logs/src/domain/configuration.ts Outdated Show resolved Hide resolved

Simplify validateAndBuildLogsConfiguration

109ce1f

amortemousque merged commit 78c8390 into main Feb 16, 2022

amortemousque deleted the aymeric/collect-other-console-logs-new branch February 16, 2022 08:17

hognevevle mentioned this pull request Mar 15, 2022

forwardConsoleLogs isn't documented #1413

Closed

BenoitZugmeyer mentioned this pull request Apr 19, 2022

✨ [RUMF-1192] forward console.* logs to Datadog #1505

Merged

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

[RUMF-1176] collect other console logs new #1316

[RUMF-1176] collect other console logs new #1316

amortemousque commented Feb 4, 2022 •

edited

Loading

codecov-commenter commented Feb 7, 2022 •

edited

Loading

BenoitZugmeyer left a comment

bcaudan Feb 8, 2022

amortemousque Feb 9, 2022

bcaudan Feb 9, 2022

amortemousque Feb 9, 2022 •

edited

Loading

amortemousque Feb 11, 2022

[RUMF-1176] collect other console logs new #1316

[RUMF-1176] collect other console logs new #1316

Conversation

amortemousque commented Feb 4, 2022 • edited Loading

Motivation

Changes

Testing

codecov-commenter commented Feb 7, 2022 • edited Loading

Codecov Report

BenoitZugmeyer left a comment

Choose a reason for hiding this comment

bcaudan Feb 8, 2022

Choose a reason for hiding this comment

amortemousque Feb 9, 2022

Choose a reason for hiding this comment

bcaudan Feb 9, 2022

Choose a reason for hiding this comment

amortemousque Feb 9, 2022 • edited Loading

Choose a reason for hiding this comment

amortemousque Feb 11, 2022

Choose a reason for hiding this comment

amortemousque commented Feb 4, 2022 •

edited

Loading

codecov-commenter commented Feb 7, 2022 •

edited

Loading

amortemousque Feb 9, 2022 •

edited

Loading