Skip to content

[FR] Allow custom attributes on built-in traces via global attribute API#8031

Open
jrodiz wants to merge 2 commits into
firebase:mainfrom
jrodiz:feature/jrc-6664.Support.custom.attributes.in.fireperf
Open

[FR] Allow custom attributes on built-in traces via global attribute API#8031
jrodiz wants to merge 2 commits into
firebase:mainfrom
jrodiz:feature/jrc-6664.Support.custom.attributes.in.fireperf

Conversation

@jrodiz
Copy link
Copy Markdown
Collaborator

@jrodiz jrodiz commented Apr 10, 2026

[FR] Allow custom attributes on built-in traces (#6664)

Summary

Resolves #6664

Firebase Performance already had putAttribute/getAttribute/removeAttribute/getAttributes on FirebasePerformance backed by a ConcurrentHashMap, but these were @hide and not wired into built-in trace serialization. This change makes them public and ensures global attributes are included on all traces and network requests — built-in and custom alike.

Changes

  • FirebasePerformance.java — Made 4 global attribute methods (putAttribute, removeAttribute, getAttribute, getAttributes) and 3 constants (MAX_TRACE_CUSTOM_ATTRIBUTES, MAX_ATTRIBUTE_KEY_LENGTH, MAX_ATTRIBUTE_VALUE_LENGTH) public
  • api.txt — Added the 4 methods to the public FirebasePerformance API surface
  • TraceMetricBuilder.java — Merges global attributes into all Trace-based metrics (screen traces, custom traces); trace-level attributes override global on key conflict
  • AppStartTrace.java — Applies global attributes to the _app_start trace metric
  • AppStateMonitor.java — Applies global attributes to foreground (_fs) and background (_bs) session traces
  • NetworkRequestMetricBuilder.java — Merges global attributes into all network request metrics (auto-instrumented OkHttp and manual HttpMetric); per-request attributes override global on key conflict

Usage

// In Application.onCreate() — set once, applies to all built-in and custom traces
FirebasePerformance.getInstance().putAttribute("build_type", "debug");
FirebasePerformance.getInstance().putAttribute("user_tier", "premium");

These attributes will appear on _app_start, _app_in_foreground, _app_in_background, screen traces (_st_*), network requests, and all custom traces automatically.

Design Notes

  • Attribute precedence: trace/request-level attributes override global attributes with the same key
  • Attribute limit: global attributes are validated at 5 max by existing checkAttribute() logic
  • Thread safety: mCustomAttributes is a ConcurrentHashMap; getAttributes() returns a defensive copy
  • Timing for _app_start: global attributes must be set before the first activity resumes (e.g., in Application.onCreate())
  • Backward compatible: all methods already existed; only visibility was changed

Test Coverage

  • _app_startAppStartTraceTest
  • _app_in_foreground (_fs) — AppStateMonitorTest
  • _app_in_background (_bs) — AppStateMonitorTest
  • Screen traces (_st_*) — AppStateMonitorTest
  • Network requests — NetworkRequestMetricBuilderTest
  • All events via ApplicationInfoTransportManagerTest

Resolves firebase#6664.

FirebasePerformance already had putAttribute/getAttribute/removeAttribute/
getAttributes backed by a ConcurrentHashMap, but they were @hide and not
wired into built-in trace serialization. This change makes them public and
ensures global attributes are included on all traces (built-in and custom).

Changes:
- Make global attribute methods (putAttribute, removeAttribute, getAttribute,
  getAttributes) public on FirebasePerformance
- Expose MAX_TRACE_CUSTOM_ATTRIBUTES, MAX_ATTRIBUTE_KEY_LENGTH,
  MAX_ATTRIBUTE_VALUE_LENGTH as public constants on FirebasePerformance
- Update api.txt with the expanded public API surface
- Merge global attributes in TraceMetricBuilder (screen + custom Trace objects)
- Add global attributes to AppStartTrace (_app_start)
- Add global attributes to AppStateMonitor.sendSessionLog() (_fs, _bs)
- Merge global attributes in NetworkRequestMetricBuilder (auto + manual HTTP)
- Trace/request-level attributes take precedence over global on key conflicts

Test coverage (TraceMetric.custom_attributes):
- AppStartTraceTest: global attrs on _app_start
- AppStateMonitorTest: global attrs on _app_in_foreground, _app_in_background,
  and screen traces (_st_*)
- TraceMetricBuilderTest: global attrs merged, trace-level overrides global
- NetworkRequestMetricBuilderTest: global attrs on network requests,
  per-request overrides global
- TransportManagerTest: global attrs on built-in traces at ApplicationInfo level
@gemini-code-assist
Copy link
Copy Markdown
Contributor

Using Gemini Code Assist

The full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips.

Invoking Gemini

You can request assistance from Gemini at any point by creating a comment using either /gemini <command> or @gemini-code-assist <command>. Below is a summary of the supported commands on the current page.

Feature Command Description

Customization

To customize Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a .gemini/ folder in the base of the repository. Detailed instructions can be found here.

Limitations & Feedback

Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counter productive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here.

@mrober mrober requested review from mrober and tejasd April 16, 2026 19:14
… full attribute scope

Global attributes apply to all traces (built-in and custom) via TraceMetricBuilder,
not only the built-in traces listed in the original changelog entry. Updated the
entry to accurately describe the full scope and mention that the attribute methods
and constants were made public.
@jrodiz
Copy link
Copy Markdown
Collaborator Author

jrodiz commented May 11, 2026

/gemini review

Copy link
Copy Markdown
Contributor

@gemini-code-assist gemini-code-assist Bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request makes global attribute methods and constants public in FirebasePerformance and ensures these attributes are merged into all traces (built-in and custom) and network requests. Feedback suggests enforcing the MAX_TRACE_CUSTOM_ATTRIBUTES limit during the merge process to prevent events from being dropped by the backend and optimizing map allocations by utilizing the defensive copy returned by getAttributes().

Comment on lines +218 to +225
Map<String, String> merged = new HashMap<>();
try {
merged.putAll(FirebasePerformance.getInstance().getAttributes());
} catch (IllegalStateException e) {
// FirebaseApp not initialized yet, skip global attributes
}
merged.putAll(attributes);
builder.clearCustomAttributes().putAllCustomAttributes(merged);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The merging logic can result in a network request having more than MAX_TRACE_CUSTOM_ATTRIBUTES (5) attributes if both global and local attributes are set. Since the backend typically enforces a limit of 5 custom attributes per event, this could lead to events being dropped or truncated. Consider enforcing the limit during the merge, giving priority to local attributes.

Comment on lines +58 to +65
Map<String, String> mergedAttributes = new HashMap<>();
try {
mergedAttributes.putAll(FirebasePerformance.getInstance().getAttributes());
} catch (IllegalStateException e) {
// FirebaseApp not initialized yet, skip global attributes
}
mergedAttributes.putAll(trace.getAttributes());
traceMetric.putAllCustomAttributes(mergedAttributes);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

high

The merging logic can result in a trace having more than MAX_TRACE_CUSTOM_ATTRIBUTES (5) attributes if both global and local attributes are set. Since the backend typically enforces a limit of 5 custom attributes per event, this could lead to events being dropped or truncated. Consider enforcing the limit during the merge, giving priority to local attributes.

Comment on lines +218 to +225
Map<String, String> merged = new HashMap<>();
try {
merged.putAll(FirebasePerformance.getInstance().getAttributes());
} catch (IllegalStateException e) {
// FirebaseApp not initialized yet, skip global attributes
}
merged.putAll(attributes);
builder.clearCustomAttributes().putAllCustomAttributes(merged);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

low

The current implementation creates a new HashMap and then copies the global attributes into it. Since FirebasePerformance.getAttributes() already returns a defensive copy (a new HashMap), you can use it directly to avoid an extra map allocation and redundant copies.

Suggested change
Map<String, String> merged = new HashMap<>();
try {
merged.putAll(FirebasePerformance.getInstance().getAttributes());
} catch (IllegalStateException e) {
// FirebaseApp not initialized yet, skip global attributes
}
merged.putAll(attributes);
builder.clearCustomAttributes().putAllCustomAttributes(merged);
Map<String, String> merged;
try {
merged = FirebasePerformance.getInstance().getAttributes();
} catch (IllegalStateException e) {
// FirebaseApp not initialized yet, skip global attributes
merged = new HashMap<>();
}
merged.putAll(attributes);
builder.clearCustomAttributes().putAllCustomAttributes(merged);

Comment on lines +58 to +65
Map<String, String> mergedAttributes = new HashMap<>();
try {
mergedAttributes.putAll(FirebasePerformance.getInstance().getAttributes());
} catch (IllegalStateException e) {
// FirebaseApp not initialized yet, skip global attributes
}
mergedAttributes.putAll(trace.getAttributes());
traceMetric.putAllCustomAttributes(mergedAttributes);
Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

low

The current implementation creates a new HashMap and then copies the global attributes into it. Since FirebasePerformance.getAttributes() already returns a defensive copy (a new HashMap), you can use it directly to avoid an extra map allocation and redundant copies.

Suggested change
Map<String, String> mergedAttributes = new HashMap<>();
try {
mergedAttributes.putAll(FirebasePerformance.getInstance().getAttributes());
} catch (IllegalStateException e) {
// FirebaseApp not initialized yet, skip global attributes
}
mergedAttributes.putAll(trace.getAttributes());
traceMetric.putAllCustomAttributes(mergedAttributes);
Map<String, String> mergedAttributes;
try {
mergedAttributes = FirebasePerformance.getInstance().getAttributes();
} catch (IllegalStateException e) {
// FirebaseApp not initialized yet, skip global attributes
mergedAttributes = new HashMap<>();
}
mergedAttributes.putAll(trace.getAttributes());
traceMetric.putAllCustomAttributes(mergedAttributes);

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

[FR]: Performance: Allow adding custom attributes to built-in traces

1 participant