Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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
feat: add insights instrumentation - events and metrics #539
feat: add insights instrumentation - events and metrics #539
Changes from all commits
d7e4a22
d8e9c46
a556188
21750be
762da65
53a1741
ff5990a
997e05e
b15d02e
b06a6dc
c11c398
da33856
3f48394
820d6e5
60dd43a
e940a76
51880d7
acb7181
1e5b671
5e0228b
7bc1a83
5dc0626
78559c5
c23f426
c4dc963
40343cb
f2d9429
fcc4ef3
9e91cd3
b83aee1
ea0101d
00892cc
85feb1f
94cd569
200154d
ed41492
16da78f
a18cd47
adfb7ca
46b6c06
d91e175
67d3e36
15a2f01
2e744f2
a2077fc
76523b0
e6446b8
dae6bd3
36530ec
874c43b
981e6f5
35bde80
a8784d0
95bbb40
121746e
521f895
9ad5e70
ee6f434
24b79be
803b7c9
dccbb7f
07efc0d
c568495
8326a19
5d84e66
8b7210c
0b7cc47
7b5c8ba
fff154d
e09d15e
4a120f6
5f90555
2676d6e
9157106
66fd331
0ec8957
File filter
Filter by extension
Conversations
Jump to
There are no files selected for viewing
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Was this supposed to be
false
by default?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a conversation about disabling it by default, but ultimately we decided to enable it by default, but only log the domain, with an optional configuration to log the full path.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Something to keep in mind for the extra optional plugin config—if it's not explicitly defined in the defaults, people won't be able to configure the plugin values via environment variables, since the env config uses the defaults to know what to look for.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh wow, thank you. This was in the back of the mind, to figure out how things are configured through ENV's.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'll add some defaults for the plugin specific configurations.