-
Notifications
You must be signed in to change notification settings - Fork 0
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
Pp 13499 add transformation #8
Conversation
This also adds an optional `service` field to the transformation to hold the name of the ECS service. The `source` for application logs is now simply `app` rather than the name of the service to improve consistency of transformation across the different types.
Replace the hard to read expected data strings with objects and then stringify them to make it easier to understand what the transformed data should look like.
function indexFromLogType(logType: CloudWatchLogTypes): string { | ||
switch(logType) { | ||
case CloudWatchLogTypes.app: | ||
return 'pay_application' | ||
case CloudWatchLogTypes['nginx-forward-proxy']: | ||
case CloudWatchLogTypes['nginx-reverse-proxy']: | ||
return 'pay_ingress' | ||
} | ||
} | ||
|
||
function extractHostFromCloudWatch(logType: CloudWatchLogTypes, data: CloudWatchRecordData): string { | ||
switch (logType) { | ||
case CloudWatchLogTypes.app: | ||
return data.logStream |
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.
You can probably leave this line out since all 3 cases fall through to the same result, although this is so unimportant so it's fine like this :)
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.
yeah you're right, if it stays like that after I add in the other types then I'll remove it 👍
function transformCloudWatchData(data: CloudWatchRecordData, envVars: EnvVars): SplunkRecord[] { | ||
return data.logEvents.map((event) => { | ||
validateLogGroup(data.logGroup) | ||
|
||
const logType: CloudWatchLogTypes = getLogTypeFromLogGroup(data.logGroup) | ||
return { | ||
host: extractHostFromCloudWatch(logType, data), | ||
source: sourceFromLogGroup(data.logGroup), | ||
source: CloudWatchLogTypes[logType], |
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'm surprised that this works, I would have expected this to be an integer, but your tests show it works, I'm just surprised :D
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.
Yeah if you just do logType.toString()
or logType
you get the integer, but the CloudWatchLogTypes[logType]
gets the string
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 used a typescript playground and went on a journey of discovery, but at least I understand why this works now:
enum Foo {
"Bar",
"Baz",
"Quux"
}
becomes
var Foo;
(function (Foo) {
Foo[Foo["Bar"] = 0] = "Bar";
Foo[Foo["Baz"] = 1] = "Baz";
Foo[Foo["Quux"] = 2] = "Quux";
})(Foo || (Foo = {}));
Incremental commit to add nginx log types and refactor the tests a little to make the expected data easier to understand. This also introduces a
service
field for cloudwatch logs concerned with an ECS service (this field needs to be added for ALB logs in the next commit).