scope -> name missing in cloudwatch logs exporter

[MaGaudin]

Component(s)

No response

What happened?

Description

Our ADOT Collector is receiving this log:
{
"resourceLogs": [
{
"resource": {
"attributes": [...]
},
"scopeLogs": [
{
"scope": {
"name": "xxxxxxx"
},
"logRecords": [
{
"timeUnixNano": "xxx",
"observedTimeUnixNano": "xxxx",
"severityNumber": 9,
"severityText": "Information",
"body": {
"stringValue": "GET / \r\n"
},
"attributes": [...],
"traceId": "xxxx",
"spanId": "xxxx"
}
]
}
]
}
]
}

but in AWS Cloudwatch we are not able to see the field scope.name. We saw from the file exporter.go (https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/exporter/awscloudwatchlogsexporter/exporter.go) that is missing the logic to retrieve the field from line 143-146.

Steps to Reproduce

Expected Result

We would like to have this field in our cloudwatch logs.

Actual Result

this field is missing

Collector version

v0.90.1 for OTEL and v0.36.0 for ADOT

Environment information

Environment

OS: (e.g., "Ubuntu 20.04")
Compiler(if manually compiled): (e.g., "go 14.2")

OpenTelemetry Collector configuration

No response

Log output

No response

Additional context

No response

[sgarzo10]

I have the same problem

[github-actions]

Pinging code owners for exporter/awscloudwatchlogs: @boostchicken @bryan-aguilar @rapphil. See Adding Labels via Comments if you do not have permissions to add labels yourself.

[crobert-1]

Hello @MaGaudin, I was able to confirm your findings and agree the scope and scope name should be included in the log being sent to CloudWatch. Code owners can correct me if there's a specific reason this should not be included.

Solution proposal:

The exporter is essentially copying all data it wants to send into a local struct called cwLogBody, which will then marshal all of the information into JSON. This JSON will then be converted to a string called Message.

I believe the solution here is to modify the function signature to accept plog.Scope as well here:

opentelemetry-collector-contrib/exporter/awscloudwatchlogsexporter/exporter.go

Line 174 in 091195a

func logToCWLog(resourceAttrs map[string]any, log plog.LogRecord, config *Config) (*cwlogs.Event, error) {

We can get Scope from sl.Scope here:

opentelemetry-collector-contrib/exporter/awscloudwatchlogsexporter/exporter.go

Line 142 in 091195a

sl := sls.At(j)

cwLogBody would also need to be modified to introduce scope here:

opentelemetry-collector-contrib/exporter/awscloudwatchlogsexporter/exporter.go

Line 162 in 091195a

type cwLogBody struct {

Then the scope information could just be added here alongside resource:

opentelemetry-collector-contrib/exporter/awscloudwatchlogsexporter/exporter.go

Line 215 in 091195a

body.Resource = resourceAttrs

Note:
At some point the code owners here may want to evaluate moving to a well-defined central marshaller for logs. I see there are some specific formatting changes being done, but having to manually copy all the data and then marshall what you've copied is prone to errors and hard to maintain. Something like what the ExporterHelper does, if that works for the use case here.

[crobert-1]

One thing to note: My proposed solution will not work when the configuration option raw_logs is set to true. I'll defer to code owners on how to handle that case, and if handling it is necessary.

[MaGaudin]

Thank you so much for the propose solution @crobert-1! So, we need to wait for an opinion from the code owners?

@boostchicken @bryan-aguilar @rapphil

[crobert-1]

You're welcome to post a PR with the change. Otherwise, yes, you'll need to wait for code owners or other contributors to provide a fix.

[bryan-aguilar]

Thank you for your investigation @crobert-1, it was very thorough and helpful. I agree with everything that has been said so far. I believe it would make sense for the InstrumentationScope to be included.

My proposed solution will not work when the configuration option raw_logs is set to true.

raw_log should really be looked through as a passthrough mode with a little logic to handle EMF messages. I think it would be okay to ignore this case.

At some point the code owners here may want to evaluate moving to a well-defined central marshaller for logs.

Agreed, this would be nice if it could fit nicely into the existing CWL logic, but I do not think and would not suggest for whoever takes on this PR to tackle this now. I would request a separate PR.

[MaGaudin]

Hi @bryan-aguilar thanks for your feedback. How could we request for a separate PR? is there any possibility to unblock this feature?

[bryan-aguilar]

I was suggesting to have separate PRs for the instrumentation scope pr and new, consistent marshaling behavior. This PR is not blocked and awaiting for a contributor to pick up the work.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• exporter/awscloudwatchlogs: @boostchicken @bryan-aguilar @rapphil

See Adding Labels via Comments if you do not have permissions to add labels yourself.

[github-actions]

This issue has been inactive for 60 days. It will be closed in 60 days if there is no activity. To ping code owners by adding a component label, see Adding Labels via Comments, or if you are unsure of which component this issue relates to, please ping @open-telemetry/collector-contrib-triagers. If this issue is still relevant, please ping the code owners or leave a comment explaining why it is still relevant. Otherwise, please close it.

Pinging code owners:

• exporter/awscloudwatchlogs: @boostchicken @bryan-aguilar @rapphil

See Adding Labels via Comments if you do not have permissions to add labels yourself.