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 more ECS url fields #181

Closed
wants to merge 4 commits into from
Closed

Add more ECS url fields #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
91754d3
Add more ECS fields for url
ChrsMark Jul 7, 2023
62b4273
Merge remote-tracking branch 'upstream/main' into add_ecs_url
ChrsMark Aug 24, 2023
e041878
review changes
ChrsMark Aug 24, 2023
ca6b587
Split content into brief and note
ChrsMark Aug 24, 2023
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
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -59,6 +59,8 @@ release.
([#252](https://github.com/open-telemetry/semantic-conventions/pull/252))
- Simplify HTTP metric briefs.
([#276](https://github.com/open-telemetry/semantic-conventions/pull/276))
- Add more ECS url fields.
([#181](https://github.com/open-telemetry/semantic-conventions/pull/181))

## v1.21.0 (2023-07-13)

Expand Down
26 changes: 26 additions & 0 deletions docs/url/url.md
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -30,6 +30,15 @@ This document defines semantic conventions that describe URL and its components.
| `url.path` | string | The [URI path](https://www.rfc-editor.org/rfc/rfc3986#section-3.3) component [2] | `/search` | Recommended |
| `url.query` | string | The [URI query](https://www.rfc-editor.org/rfc/rfc3986#section-3.4) component [3] | `q=OpenTelemetry` | Recommended |
| `url.fragment` | string | The [URI fragment](https://www.rfc-editor.org/rfc/rfc3986#section-3.5) component | `SemConv` | Recommended |
| `url.registered_domain` | string | The highest registered url domain, stripped of the subdomain. [4] | `example.com` | Opt-In |
| `url.subdomain` | string | The subdomain portion of a fully qualified domain name includes all of the names except the host name under the registered_domain. In a partially qualified domain, or if the the qualification level of the full name cannot be determined, subdomain contains all of the names below the registered domain. [5] | `east` | Opt-In |
| `url.top_level_domain` | string | The effective top level domain (eTLD), also known as the domain suffix, is the last part of the domain name. For example, the top level domain for example.com is "com". [6] | `co.uk` | Opt-In |
| `url.username` | string | Username of the request. | `user42` | Opt-In |
| `url.password` | string | Password of the request. | `changeme` | Opt-In |
Copy link
Contributor

Choose a reason for hiding this comment

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

We should definitely add some caveat here about sensitive information :)

I forget how we decided to call this out or if that's still TBD, but I'd love a not on these fields about sensitivity.

| `url.extension` | string | The field contains the file extension from the original request url, excluding the leading dot. [7] | `png` | Opt-In |
| `url.domain` | string | Domain of the url, such as `www.opentelemetry.io`. [8] | `www.opentelemetry.io` | Opt-In |
Copy link
Member

@Oberon00 Oberon00 Aug 29, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This is redundant with server.address (or server.domain as proposed in #290).

Copy link
Member

@AlexanderWert AlexanderWert Sep 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

@Oberon00 Thanks for the review!

I'd like to use your comment to address two aspects here: one that is directly related to this proposal here and one general aspect.

This url.* attributes proposal

While it's true that at a first glance that looks redundant, I think there is reason for existence for both, because of the following:

  1. The semantics (or the context) in which these attributes are used can be slightly different. url.* fields describe a URL. server/client.address/domain attributes describe a client-server relationship. There are cases (that ECS users already rely on) in which url.* fields are used, including security use cases (e.g. analyzing access logs and their patterns for security patterns) that are different to the server/client.* fields.
  2. This proposal here only defines attributes that are NOT set into a specific semantic convention context, yet. So they are not part of the HTTP semantic conventions and do not conflict or imply redundancy on the HTTP semantic conventions (where server.address, etc. are used).
  3. This PR is basically an implementation step of what we agreed on in Support Elastic Common Schema (ECS) in OpenTelemetry oteps#222

Seeking general agreement

I'd like us to come to a general agreement regarding open-telemetry/oteps#222 on the following:

  • When adding ECS fields to semantic attributes that do not conflict with existing attributes in the context of individual semantic conventions, let's be less strict on the following aspects, with the goal to move faster with the ECS merger (otherwise it will take very long for us to get ECS in):
    • redundancy arguments
    • syntactical attribute naming rules (e.g. plural vs. singular)
    • naming preferences
    • other "soft" arguments
  • I think my proposal Proposal: Decoupling Attribute definition from Attribute usage in models #197 would help with that as it decouples definition of attributes (which we can do rather uncomplicated) from usage in concrete semantic conventions.
  • Let's consider ECS fields and entire field groups that are not covered by OTel semconv, yet, as "low hanging fruits" for merging ECS into OTel and move fast with those (as there are not conflicts and those are practice-proven). So, let's try to avoid discussions related to the above-mentioned arguments on each of the PRs we'll have regarding adding ECS fields to OTel. (Note: I'm not implying that we should not have discussions at all in those case where it makes sense, just trying to avoid similar discussions to repeat.)

These are the reasons for the above proposal:

  • In Support Elastic Common Schema (ECS) in OpenTelemetry oteps#222 we agreed on merging ECS with OTel, so the faster we do at least the low hanging fruits the better and less friction for the communities (ECS and OTel).
  • A big community is relying on ECS fields today, and there are many practice-proven use cases for the existing ECS fields (that will become OTel use cases in the future as well). So whenever possible we should avoid breaking changes (including dropping fields) on both sides (OTel and ECS). The above case is a great example of when it doesn't hurt to add these fields to OTel (because there are no conflicts and we are not proposing to use them in the HTTP semconv, so even redundancy is not a reason). But, for ECS users it would be a huge breaking change to drop the fields.
  • The biggest value of Support Elastic Common Schema (ECS) in OpenTelemetry oteps#222 is to merge the ECS and OTel communities into one community with all its mutually additional use cases. However, that only works if we do not build everything from scratch but try to keep (on both ECS and OTel semconv) things that were proven in practice (in case there are not conflicts and breaking changes can be avoided).
  • The faster we merge ECS into OTel the less friction there is for the community and the faster we can start expanding OTel with ECS use cases.

@open-telemetry/specs-semconv-approvers @open-telemetry/specs-semconv-maintainers WDYT about the above general proposal? If it helps I can extract it into a separate issue, or so?

Copy link
Member

@Oberon00 Oberon00 Sep 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

The semantics (or the context) in which these attributes are used can be slightly different.

Emphasis on "slightly". There is server.address for the "logical" address and server.socket.domain for the physical address. We will probably also get server.domain. Now this PR proposes a third (fourth) attribute url.domain. I understand that this is sensible for ECS compatibility reasons. Could we mark the attribute as "compatibility-only"? Specific semantic conventions can have (and do have) more specific usage instructions, so I believe there is really no need from semantic convention designer side for this new attribute.

Although you say:

There are cases (that ECS users already rely on) in which url.* fields are used, including security use cases (e.g. analyzing access logs and their patterns for security patterns) that are different to the server/client.* fields.

I would be interested to know more. It sound like in these cases, what is stored in server.* might be better stored in server.socket.*?

Copy link
Member

Choose a reason for hiding this comment

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

I filed #329 with some specific guidance I would like to have re: adding ECS conventions to OTel

| `url.port` | int | Port of the request | `9090` | Opt-In |
Copy link
Member

Choose a reason for hiding this comment

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

This is redundant with server.port.

| `url.original` | string | Unmodified original URL as seen in the event source. [9] | `https://www.opentelemetry.io/search/?q=container` | Opt-In |
Copy link
Member

Choose a reason for hiding this comment

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

It is unclear how url.full should be modified.


**[1]:** For network calls, URL usually has `scheme://host[:port][path][?query][#fragment]` format, where the fragment is not transmitted over HTTP, but if it is known, it should be included nevertheless.
`url.full` MUST NOT contain credentials passed via URL in form of `https://username:password@www.example.com/`. In such case username and password should be redacted and attribute's value should be `https://REDACTED:REDACTED@www.example.com/`.
Expand All @@ -38,6 +47,23 @@ This document defines semantic conventions that describe URL and its components.
**[2]:** When missing, the value is assumed to be `/`

**[3]:** Sensitive content provided in query string SHOULD be scrubbed when instrumentations can identify it.

**[4]:** For example, the registered domain for "foo.example.com" is "example.com".
This value can be determined precisely with a list like the public suffix list (`http://publicsuffix.org`). Trying to approximate this by simply taking the last two labels will not work well for TLDs such as "co.uk".

**[5]:** For example the subdomain portion of `www.east.mydomain.co.uk` is "east". If the domain has multiple levels of subdomain, such as `sub2.sub1.example.com`, the subdomain field should contain "sub2.sub1", with no trailing period.

**[6]:** This value can be determined precisely with a list like the public suffix list (`http://publicsuffix.org`). Trying to approximate this by simply taking the last label will not work well for effective TLDs such as `co.uk`.

**[7]:** The file extension is only set if it exists, as not every url has a file extension.
The leading period must not be included. For example, the value must be "png", not ".png".
Note that when the file name has multiple extensions (example.tar.gz), only the last one should be captured ("gz", not "tar.gz").

**[8]:** In some cases a URL may refer to an IP and/or port directly, without a domain name. In this case, the IP address would go to the domain field.
If the URL contains a literal IPv6 address enclosed by [ and ] (IETF RFC 2732), the [ and ] characters should also be captured in the domain field.

**[9]:** Note that in network monitoring, the observed URL may be a full URL, whereas in access logs, the URL is often just represented as a path.
This field is meant to represent the URL as it was observed, complete or not.
<!-- endsemconv -->

## Sensitive information
Expand Down
90 changes: 90 additions & 0 deletions model/url.yaml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -37,3 +37,93 @@ groups:
type: string
brief: 'The [URI fragment](https://www.rfc-editor.org/rfc/rfc3986#section-3.5) component'
examples: ["SemConv"]
- id: registered_domain
requirement_level: opt_in
type: string
brief: >
Copy link
Member

@joaopgrassi joaopgrassi Aug 16, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

You need to use brief: | here to be able to achieve multiple lines I think. Right now, the table is broken into multiple lines

image

It's also the same in the other attributes

Copy link
Member Author

Choose a reason for hiding this comment

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

Even with that the table was kinda broken. I moved some content into the note section and only kept the basic definitions in the brief.

The highest registered url domain, stripped of the subdomain.
note: >
For example, the registered domain for "foo.example.com" is "example.com".

This value can be determined precisely with a list like the public suffix
list (`http://publicsuffix.org`). Trying to approximate this by simply taking
the last two labels will not work well for TLDs such as "co.uk".
examples: [ "example.com" ]
- id: subdomain
requirement_level: opt_in
type: string
brief: >
The subdomain portion of a fully qualified domain name includes all of
the names except the host name under the registered_domain. In a partially
qualified domain, or if the the qualification level of the full name cannot
be determined, subdomain contains all of the names below the registered domain.
note: >
For example the subdomain portion of `www.east.mydomain.co.uk` is "east".
If the domain has multiple levels of subdomain, such as `sub2.sub1.example.com`,
the subdomain field should contain "sub2.sub1", with no trailing period.
examples: [ "east" ]
- id: top_level_domain
requirement_level: opt_in
type: string
brief: >
The effective top level domain (eTLD), also known as the domain suffix,
is the last part of the domain name. For example, the top level domain
for example.com is "com".
note: >
This value can be determined precisely with a list like the public suffix list
(`http://publicsuffix.org`). Trying to approximate this by simply taking the last
label will not work well for effective TLDs such as `co.uk`.
examples: [ "co.uk" ]
- id: username
Copy link
Member

Choose a reason for hiding this comment

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

Should we add username and password? @trask linked to them being deprecated? https://www.ietf.org/rfc/rfc3986.txt

Copy link
Member Author

@ChrsMark ChrsMark Aug 24, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Hmm, I was not aware of those being deprecated.
@AlexanderWert @trisch-me do you you know more about this and if those would be deprecated from ECS?

Copy link
Member

Choose a reason for hiding this comment

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

Instrumentations also cover older http clients which would still contain username and password.
So, I think username and password could still be useful as opt-in attributes.

requirement_level: opt_in
type: string
brief: Username of the request.
examples: [ "user42" ]
- id: password
requirement_level: opt_in
type: string
brief: Password of the request.
examples: [ "changeme" ]
- id: extension
requirement_level: opt_in
type: string
brief: >
The field contains the file extension from the original request url,
excluding the leading dot.
note: >
The file extension is only set if it exists, as not every url has
a file extension.

The leading period must not be included. For example, the value must
be "png", not ".png".

Note that when the file name has multiple extensions (example.tar.gz),
only the last one should be captured ("gz", not "tar.gz").
examples: [ "png" ]
- id: domain
requirement_level: opt_in
type: string
brief: >
Domain of the url, such as `www.opentelemetry.io`.
note: >
In some cases a URL may refer to an IP and/or port directly,
without a domain name. In this case, the IP address would go to the domain field.

If the URL contains a literal IPv6 address enclosed by [ and ] (IETF RFC 2732),
the [ and ] characters should also be captured in the domain field.
examples: [ "www.opentelemetry.io" ]
- id: port
requirement_level: opt_in
type: int
brief: Port of the request
examples: [ 9090 ]
- id: original
requirement_level: opt_in
type: string
brief: Unmodified original URL as seen in the event source.
note: >
Note that in network monitoring, the observed URL may be
a full URL, whereas in access logs, the URL is often just represented as a path.

This field is meant to represent the URL as it was observed, complete or not.
examples: [ "https://www.opentelemetry.io/search/?q=container" ]
Loading