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.

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

Clarify the mapping between Google Cloud Logging and OpenTelemetry #3775

Closed
wants to merge 2 commits into from

Conversation

alexvanboxel
Copy link

Changed

  • Clarify that the JSON representation is used for the LogEntry
  • Change to Pascal-case for representing the field in LogEntry
  • Map MonitoredResource to resource attributes with gcp prefix
  • Clarify the type of the Body for jsonPayload, protoPayload, and textPayload

Added

  • Mapping section for Severity from Cloud Logging to OpenTelemetry
  • Mapping section for Severity from OpenTelemetry to Cloud Logging
  • Semantic Mapping section (only includes log.record.uid)
  • Clarified that the JSON representation is used for the LogEntry
  • Map LogEntryOperation to resource attribute with gcp.operation (KVList)
  • Map LogEntrySourceLocation to resource attribute with gcp.source_location (KVList)
  • Map LogSplit to resource attribute with gcp.log_split (KVList)

Why

With this draft collector PR, the PubsubReceiver will be able to handle the native translation of Google Cloud LogEntry's. But translations are opinionated. I prefer that we agree on the target mapping before merging the PR.

Fixes #

Related issues:

#3774
open-telemetry/opentelemetry-collector-contrib#23184

Draft PR in the Collector:
open-telemetry/opentelemetry-collector-contrib#29299

Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@carlosalberto
Copy link
Contributor

Any reason this is a draft PR? Are you waiting specific feedback from somebody?

cc @jsuereth @dashpole

@alexvanboxel
Copy link
Author

I'm waiting for feedback on the linked issue (as described in the PR process). We can have a conversation here if that works better.

Copy link

github-actions bot commented Dec 5, 2023

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 5, 2023
* Clarify that the JSON representation is used for the LogEntry
* Change to Pascal-case for representing the field in LogEntry
* Map MonitoredResource to resource attributes with gcp prefix
* Clarify the type of the Body for jsonPayload, protoPayload, and textPayload

* Mapping section for Severity from Cloud Logging to OpenTelemetry
* Mapping section for Severity from OpenTelemetry to Cloud Logging
* Semantic Mapping section (only includes log.record.uid)
* Clarified that the JSON representation is used for the LogEntry
* Map LogEntryOperation to resource attribute with gcp.operation (KVList)
* Map LogEntrySourceLocation to resource attribute with gcp.source_location (KVList)
* Map LogSplit to resource attribute with gcp.log_split (KVList)

With this [draft collector PR](open-telemetry/opentelemetry-collector-contrib#29299),
the PubsubReceiver will be able to handle the native translation of Google Cloud LogEntry's.
But translations are opinionated. I prefer that we agree on the target mapping before merging
the PR.
@alexvanboxel
Copy link
Author

I'm pushing the implementation in the collector, so it makes sense to have a compatibility section. I've marked the implementation as alpha so we still can do some braking changes if needed.

@github-actions github-actions bot removed the Stale label Dec 16, 2023
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 23, 2023
@alexvanboxel
Copy link
Author

not stale... waiting till open-telemetry/opentelemetry-collector-contrib#29299 gets merged...

@github-actions github-actions bot removed the Stale label Dec 24, 2023
Copy link

This PR was marked stale due to lack of activity. It will be closed in 7 days.

@github-actions github-actions bot added the Stale label Dec 31, 2023
Copy link

github-actions bot commented Jan 7, 2024

Closed as inactive. Feel free to reopen if this PR is still being worked on.

@github-actions github-actions bot closed this Jan 7, 2024
Copy link

@kamalmarhubi kamalmarhubi left a comment

Choose a reason for hiding this comment

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

@alexvanboxel unsure if you plan to keep this moving, but I added a couple of comments just in case

| `resource` | `MonitoredResource` | The monitored resource that produced this log entry. | `Resource["gcp.*"]` |
| `httpRequest` | `HttpRequest` | The HTTP request associated with the log entry, if any. | `Attributes["gcp.http_request"]` (KVList) |
| `operation` | `LogEntryOperation` | Information about a operation associated with the log entry. | `Attributes["gcp.operation"]` (KVList) |
| `sourceLocation` | `LogEntrySourceLocation` | Source code location information associated with the log entry. | `Attributes["gcp.source_location"]` (KVList) |

Choose a reason for hiding this comment

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

looking at docs for LogEntrySourceLocation I this ought to be mapped to the equivalent semconvcode.* attributes as follows:

  • sourceLocation.file => code.filepath
  • sourceLocation.line => code.lineno

Less sure of what to do with sourceLocation.function, which is described as

Human-readable name of the function or method being invoked, with optional context such as the class or package name. This information may be used in contexts such as the logs viewer, where a file and line number are less meaningful. The format can vary by language. For example: qual.if.ied.Class.method (Java), dir/package.func (Go), function (Python).

it ought to go into the code.namespace and code.function attributes. if you happen to have a sample of data across different languages for which gcp has a logging integration, perhaps the right thing to do can be figured out based on that? failing that, a reasonable best effort might be (python style split / indexing):

  • code.function gets sourceLocation.function.split(".")[-1]
  • code.namespace gets sourceLocation.function.split(".")[:-1]

| `traceSampled` | `boolean` | The sampling decision of the trace associated with the log entry. | `TraceFlags.SAMPLED` |
| `labels` | `map<string,string>` | A set of user-defined (key, value) data that provides additional information about the log entry. | `Attributes` |
| `resource` | `MonitoredResource` | The monitored resource that produced this log entry. | `Resource["gcp.*"]` |
| `httpRequest` | `HttpRequest` | The HTTP request associated with the log entry, if any. | `Attributes["gcp.http_request"]` (KVList) |

Choose a reason for hiding this comment

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

could be worth attempting to map these to http semconv? not sure why I didn't do it initially.

  • requestMethod => http.request.method
  • requestUrl => url.full
  • requestSize => see [0] below
  • status => http.response.status_code
  • responseSize => see [0] below
  • userAgent => user_agent.original
  • remoteIp => client.address
  • serverIp => server.address
  • referer => http.request.header.referer[0]
  • protocol => net.protocol.name (lower case), net.protocol.version

The remaining ones can be mapped to gcp.*:

  • latency
  • cacheLookup
  • cacheHit
  • cacheValidatedWithOriginServer
  • cacheFillBytes

[0] I had previously opened open-telemetry/semantic-conventions#84 to add "whole-request" size attributes, ie headers + body. I will rebase / update the PR this week to see if it gets any traction.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants