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

Add redis resource #2836

Closed
wants to merge 2 commits into from
Closed
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -532,6 +532,8 @@ release.
conventions. ([#2272](https://github.com/open-telemetry/opentelemetry-specification/pull/2272))
- Add opentracing.ref_type semantic convention.
([#2297](https://github.com/open-telemetry/opentelemetry-specification/pull/2297))
- Add `db.redis.instance` Resource attribute in Redis.
Copy link
Author

Choose a reason for hiding this comment

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

I'll add this PR here before moving out of draft
#2836

([#2145](https://github.com/open-telemetry/opentelemetry-specification/pull/2145))

### Compatibility

Expand Down
14 changes: 14 additions & 0 deletions semantic_conventions/resource/db/redis.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,14 @@
groups:
- id: redis
Copy link
Member

@joaopgrassi joaopgrassi Oct 4, 2022

Choose a reason for hiding this comment

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

Isn't this PR having the same "issue" as the original PR: #2145 (comment)

Did you use this approach as a "workaround" for the conflict as here #2145 (comment)

or is the intention to have redis as a "top-level" entry? If it's because it's a workaround, then maybe we should look into the generator more, or find another way to generate it.

prefix: db.redis
brief: >
A redis instance
attributes:
- id: instance
type: string
brief: >
The reported name of the Redis instance. This can be in the form of
`{host}:{port}` or any other name provided manually while configuring
the instrumentation. If not provided, the default value is the `endpoint`
value provided in the configuration.
examples: ['localhost:6379', 'product_info_redis']
7 changes: 7 additions & 0 deletions specification/resource/semantic_conventions/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ This document defines standard attributes for resources. These attributes are ty
- [Compute Unit](#compute-unit)
- [Compute Instance](#compute-instance)
- [Environment](#environment)
- [Database](#database)
- [Version attributes](#version-attributes)
- [Cloud-Provider-Specific Attributes](#cloud-provider-specific-attributes)

Expand Down Expand Up @@ -158,6 +159,12 @@ Attributes defining a running environment (e.g. Operating System, Cloud, Data Ce
- [Kubernetes](./k8s.md)
- [Browser](./browser.md)

## Database

Attributes defining a database service.

- [Redis](./db/redis.md)

## Version attributes

Version attributes, such as `service.version`, are values of type `string`. They are
Expand Down
13 changes: 13 additions & 0 deletions specification/resource/semantic_conventions/db/redis.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
# Redis
Copy link
Member

Choose a reason for hiding this comment

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

We already have Redis-specific conventions in database.md. Do we need to introduce a new file just for Redis?

Copy link
Author

Choose a reason for hiding this comment

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

After talking with Joao and verifying on my end, it looks like you're correct and we can indeed use the existing file even though it's under a different "path" with the "traces".


**Status**: [Experimental](../../../document-status.md)

**type:** `db.redis`

**Description:** A redis instance

<!-- semconv redis -->
| Attribute | Type | Description | Examples | Required |
|---|---|---|---|---|
| `db.redis.instance` | string | The reported name of the Redis instance. This can be in the form of `{host}:{port}` or any other name provided manually while configuring the instrumentation. If not provided, the default value is the `endpoint` value provided in the configuration. | `localhost:6379`; `product_info_redis` | No |
<!-- endsemconv -->