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

Shared attributes prototype #4045

Closed
wants to merge 4 commits into from

Conversation

martinkuba
Copy link
Contributor

!!! DO NOT MERGE !!!
(intended for discussion only)

Which problem is this PR solving?

This is a prototype of shared/common attributes - attributes that are applied to all spans and log records. The immediate use cases are for the Web SDK where a number of attributes are relevant across many traces and events. Example attributes are session ID, page URL, window width/height.

Background

Originally, we wanted to capture these as resource attributes (see this OTEP), but it has become apparent that this would likely not get accepted.

Another alternative proposed solution was to capture these as "context" attributes (see this OTEP). This prototype includes a mechanism for context attributes. However, the primary use case for client telemetry is global attributes, which cannot be implemented using context.

Short description of the changes

The prototype introduces a new package shared-attributes, which includes

  • APIs for capturing and retrieving global attributes - setGlobalAttributes(), getGlobalAttributes()
  • Span and LogRecord processors that call getGlobalAttributes() and add the attributes to each signal
  • same as above but for context attributes
    • setContextAttributes(), getContextAttributes()
    • Span and LogRecord processors for context attributes

The main files to focus on are:

This prototype also comes with a web example app that shows how it might be used

Open Questions

  • Do we want to have a generic mechanism like this for capturing global attributes (as opposed to handling each attribute case by case)?
  • Where should these APIs live? Does a separate package like this make sense or should this be part of the core SDK?
  • Is this something that all SDKs should adopt, and does it require a spec?

@Flarna
Copy link
Member

Flarna commented Aug 9, 2023

As there were already spec discussions created by people with no JS focus it's a clear indication that this is likely relevant for other languages as well.

The context attributes seem to be well scoped.

For the global attributes I'm not that sure. They might change several time while spans are started/ended in parallel. A snapshot is copied to span at start but changes afterwards are not seen. As I don't have a specific use case in mind for these attributes it's hard to judge if such races are problematic.

First thing I miss is a remove/clear API but well, that's easy to add.

I think this needs to be on API level. In special the context attributes are likely created/modified by instrumentations because these are the locations where e.g. a span is set on a context and a context is activated for a specific code range.
Till now it was quite clear that instrumentations should only depend on API.
I know there is core which is needed in some cases but I remember several discussions in this area which mostly ended up in core should be moved to API.

One topic to clarify is likely the priority of attributes. Which value ends up in the span if the same key is used global, on context and directly set?

@github-actions
Copy link

github-actions bot commented Oct 9, 2023

This PR is stale because it has been open 60 days with no activity. Remove stale label or comment or this will be closed in 14 days.

@github-actions github-actions bot added the stale label Oct 9, 2023
@github-actions
Copy link

This PR was closed because it has been stale for 14 days with no activity.

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.

2 participants