Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Add modify_fields processor and use it to set logName #474

Merged
merged 55 commits into from
Apr 14, 2022
Merged

Add modify_fields processor and use it to set logName #474

merged 55 commits into from
Apr 14, 2022

Conversation

quentinmit
Copy link
Member

@quentinmit quentinmit commented Mar 17, 2022 •
edited
Loading

Major changes in this branch:

  • Refactor exclude_logs and filter parsing to be more consistent with Cloud Logging syntax
  • Use Lua for most exclude_logs and modify_fields conditions
  • Add modify_fields processor
    • Source value: move_from, copy_from, static_value
    • default_value if source value is unset
    • map_values, type to transform the values
    • omit_if to conditionally remove a field
  • Set logName using the new processor

quentinmit added 22 commits March 17, 2022 15:50
@quentinmit
First pass at converting Targets to Lua expressions
68f1f7f
@quentinmit
Make it build
26e4c17
@quentinmit
Generate and test arbitrary conf file sets
e28c4e8
@quentinmit
Add modify_fields processor
a131440
@quentinmit
Print golden diffs line-by-line for readability
9c37654
@quentinmit
Insert Lua code into filter chain
af082fd
@quentinmit
Add cleanup code
b179d08
@quentinmit
Implement map_values and type
5fd2fd0
@quentinmit
Generate Lua expressions for filters whenever possible
e3c5d66
@quentinmit
Use Lua code to evaluate filters whenever possible
583a511
@quentinmit
Fix missing end in modify_fields
98c7296
@quentinmit
Make update_golden remove files as well
eefba52
@quentinmit
Update golden files for processor_exclude_logs test
137280d
@quentinmit
Fix Lua not-equals operator
dc729e5
@quentinmit
More != fixes
522d8aa
@quentinmit
Remove unused code
1b57b3b
@quentinmit
Don't use modify filter with Copy because it doesn't support record a…
bf0783e 
…ccessors
@quentinmit
Don't look in record for local variables
f59eed8
@quentinmit
Remove unused cleanup code
d0f0c57
@quentinmit
Set logName using Lua
083fb7c
@quentinmit
Report instrumentation source as a label
32cfbc3
@quentinmit
Update golden files
46200fb
@quentinmit quentinmit requested a review from qingling128 March 17, 2022 20:27
@quentinmit quentinmit force-pushed the quentin-logtransform branch from c3fb21d to cc99f52 Compare March 17, 2022 22:38
@quentinmit quentinmit force-pushed the quentin-logtransform branch from 2ade241 to 3c9748a Compare March 21, 2022 18:46
@quentinmit quentinmit force-pushed the quentin-logtransform branch from f03f58a to 8ae8016 Compare April 8, 2022 20:18
@quentinmit quentinmit marked this pull request as ready for review April 12, 2022 19:14
jefferbrecht
jefferbrecht reviewed Apr 13, 2022
View reviewed changes
confgenerator/filter/internal/ast/ast.go Outdated
// Cloud Logging's filter syntax has extremely weird rules around quoting/escaping
//
// A "text" is a literal that is not surrounded by double quotes. A
// test may contain a small number of special characters, must start
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
// test may contain a small number of special characters, must start
// text may contain a small number of special characters, must start

qingling128
qingling128 approved these changes Apr 13, 2022
View reviewed changes
Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

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

Did another pass and resolved some open threads. The only remaining things:

LGTM modulo the two things above are addressed.

qingling128
qingling128 approved these changes Apr 14, 2022
View reviewed changes
Copy link
Contributor

@qingling128 qingling128 left a comment

Choose a reason for hiding this comment

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

LGTM

@quentinmit quentinmit changed the title Add modify_fields processor and use it to set logName and instrumentation_source Add modify_fields processor and use it to set logName Apr 14, 2022
@quentinmit quentinmit merged commit 1b1cf03 into master Apr 14, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jefferbrecht jefferbrecht jefferbrecht approved these changes

@qingling128 qingling128 qingling128 approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@quentinmit @qingling128 @jefferbrecht