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

[receiver/mongodbatlasreceiver] added mongodb_atlas.user.alias attribute, the user-friendly hostname of the cluster node as displayed in the Atlas portal. #18881

Merged
merged 1 commit into from
Apr 11, 2023

Conversation

dloucasfx
Copy link
Contributor

Description:
The MongoDB Atlas Portal does not display the internal hostname of the shard (ie: mongodb_atlas.host.name attribute), it uses the User Alias property instead.

Portal:
Screen Shot 2023-02-22 at 11 26 33 AM

Note the process->hostname attribute is totally different:
Screen Shot 2023-02-22 at 11 26 58 AM

This creates confusion for the user.
In this PR, I am introducing a new attribute mongodb_atlas.user.alias

Link to tracking Issue:

Testing:
Manual validation against a trial account

Resource attributes:
     -> mongodb_atlas.org_name: Str(Splunk)
     -> mongodb_atlas.project.name: Str(Project 0)
     -> mongodb_atlas.project.id: Str(63ee7fba9dc87e12910b7b4b)
     -> mongodb_atlas.host.name: Str(atlas-wt8qir-shard-00-02.p11f4l5.mongodb.net)
     -> mongodb_atlas.user.alias: Str(ac-omq9hhf-shard-00-02.p11f4l5.mongodb.net)
     -> mongodb_atlas.process.port: Str(27017)
     -> mongodb_atlas.process.type_name: Str(REPLICA_PRIMARY)
     -> mongodb_atlas.process.id: Str(atlas-wt8qir-shard-00-02.p11f4l5.mongodb.net:27017)
     -> mongodb_atlas.disk.partition: Str(data)
     -> host.name: Str(C02DN2DQMD6P)
     -> os.type: Str(darwin)
     -> host.id: Str(DA88CF9C-03E9-5C08-8E92-98B8B97D56B1)

Documentation:
auto generated

@runforesight
Copy link

runforesight bot commented Feb 22, 2023

Foresight Summary

    
Major Impacts

build-and-test-windows duration(6 seconds) has decreased 30 minutes 38 seconds compared to main branch avg(30 minutes 44 seconds).
View More Details

⭕  build-and-test-windows workflow has finished in 6 seconds (30 minutes 38 seconds less than main branch avg.) and finished at 10th Apr, 2023.


Job Failed Steps Tests
windows-unittest-matrix -     🔗  N/A See Details
windows-unittest -     🔗  N/A See Details

✅  telemetrygen workflow has finished in 1 minute 1 second and finished at 10th Apr, 2023.


Job Failed Steps Tests
build-dev -     🔗  N/A See Details
publish-latest -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details

✅  check-links workflow has finished in 1 minute 46 seconds (⚠️ 47 seconds more than main branch avg.) and finished at 10th Apr, 2023.


Job Failed Steps Tests
changed files -     🔗  N/A See Details
check-links -     🔗  N/A See Details

✅  changelog workflow has finished in 2 minutes 3 seconds and finished at 10th Apr, 2023.


Job Failed Steps Tests
changelog -     🔗  N/A See Details

✅  prometheus-compliance-tests workflow has finished in 3 minutes 10 seconds (3 minutes 9 seconds less than main branch avg.) and finished at 10th Apr, 2023.


Job Failed Steps Tests
prometheus-compliance-tests -     🔗  N/A See Details

✅  load-tests workflow has finished in 6 minutes 40 seconds (3 minutes 39 seconds less than main branch avg.) and finished at 10th Apr, 2023.


Job Failed Steps Tests
setup-environment -     🔗  N/A See Details
loadtest (TestIdleMode) -     🔗  N/A See Details
loadtest (TestBallastMemory|TestLog10kDPS) -     🔗  N/A See Details
loadtest (TestMetric10kDPS|TestMetricsFromFile) -     🔗  N/A See Details
loadtest (TestMetricResourceProcessor|TestTrace10kSPS) -     🔗  N/A See Details
loadtest (TestTraceNoBackend10kSPS|TestTrace1kSPSWithAttrs) -     🔗  N/A See Details
loadtest (TestTraceBallast1kSPSWithAttrs|TestTraceBallast1kSPSAddAttrs) -     🔗  N/A See Details
loadtest (TestTraceAttributesProcessor) -     🔗  N/A See Details

✅  e2e-tests workflow has finished in 13 minutes 32 seconds and finished at 10th Apr, 2023.


Job Failed Steps Tests
kubernetes-test (v1.26.0) -     🔗  N/A See Details
kubernetes-test (v1.25.3) -     🔗  N/A See Details
kubernetes-test (v1.24.7) -     🔗  N/A See Details
kubernetes-test (v1.23.13) -     🔗  N/A See Details

✅  build-and-test workflow has finished in 37 minutes (9 minutes 36 seconds less than main branch avg.) and finished at 10th Apr, 2023.


Job Failed Steps Tests
setup-environment -     🔗  N/A See Details
check-codeowners -     🔗  N/A See Details
lint-matrix (receiver-0) -     🔗  N/A See Details
lint-matrix (receiver-1) -     🔗  N/A See Details
lint-matrix (processor) -     🔗  N/A See Details
lint-matrix (exporter) -     🔗  N/A See Details
lint-matrix (extension) -     🔗  N/A See Details
lint-matrix (connector) -     🔗  N/A See Details
lint-matrix (internal) -     🔗  N/A See Details
lint-matrix (other) -     🔗  N/A See Details
checks -     🔗  N/A See Details
check-collector-module-version -     🔗  N/A See Details
correctness-metrics -     🔗  N/A See Details
correctness-traces -     🔗  N/A See Details
integration-tests -     🔗  N/A See Details
unittest-matrix (1.20, receiver-0) -     🔗  N/A See Details
unittest-matrix (1.20, receiver-1) -     🔗  N/A See Details
unittest-matrix (1.20, processor) -     🔗  N/A See Details
unittest-matrix (1.20, exporter) -     🔗  N/A See Details
unittest-matrix (1.20, extension) -     🔗  N/A See Details
unittest-matrix (1.20, connector) -     🔗  N/A See Details
unittest-matrix (1.20, internal) -     🔗  N/A See Details
unittest-matrix (1.20, other) -     🔗  N/A See Details
unittest-matrix (1.19, receiver-0) -     🔗  N/A See Details
unittest-matrix (1.19, receiver-1) -     🔗  N/A See Details
unittest-matrix (1.19, processor) -     🔗  N/A See Details
unittest-matrix (1.19, exporter) -     🔗  N/A See Details
unittest-matrix (1.19, extension) -     🔗  N/A See Details
unittest-matrix (1.19, connector) -     🔗  N/A See Details
unittest-matrix (1.19, internal) -     🔗  N/A See Details
unittest-matrix (1.19, other) -     🔗  N/A See Details
build-examples -     🔗  N/A See Details
lint -     🔗  N/A See Details
unittest (1.20) -     🔗  N/A See Details
unittest (1.19) -     🔗  N/A See Details
cross-compile (darwin, amd64) -     🔗  N/A See Details
cross-compile (darwin, arm64) -     🔗  N/A See Details
cross-compile (linux, 386) -     🔗  N/A See Details
cross-compile (linux, amd64) -     🔗  N/A See Details
cross-compile (linux, arm) -     🔗  N/A See Details
cross-compile (linux, arm64) -     🔗  N/A See Details
cross-compile (linux, ppc64le) -     🔗  N/A See Details
cross-compile (windows, 386) -     🔗  N/A See Details
cross-compile (windows, amd64) -     🔗  N/A See Details
build-package (deb) -     🔗  N/A See Details
build-package (rpm) -     🔗  N/A See Details
windows-msi -     🔗  N/A See Details
publish-check -     🔗  N/A See Details
publish-stable -     🔗  N/A See Details
publish-dev -     🔗  N/A See Details
rotate-milestone -     🔗  N/A See Details

🔎 See details on Foresight

*You can configure Foresight comments in your organization settings page.

Copy link
Member

@djaglowski djaglowski left a comment

Choose a reason for hiding this comment

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

This would be considered a breaking change, since some backends will track metrics based on their full set of resource attributes. We'd need to enable this through the feature gate process described here.

@djaglowski
Copy link
Member

cc: @schmikei

@dloucasfx
Copy link
Contributor Author

This would be considered a breaking change, since some backends will track metrics based on their full set of resource attributes. We'd need to enable this through the feature gate process described here.

@djaglowski can you expand on why this would be considered a breaking change. I am not removing or renaming any attribute, I am just adding an extra attribute.
At least here at splunk, additional attributes on the MTS will not break the filtering logic.
Thanks!

@dloucasfx dloucasfx changed the title [receiver/mongodbatlasreceiver] added mongodb_atlas.user.alias attrinute, the user-friendly hostname of the cluster node as displayed in the Atlas portal. [receiver/mongodbatlasreceiver] added mongodb_atlas.user.alias attribute, the user-friendly hostname of the cluster node as displayed in the Atlas portal. Feb 22, 2023
@djaglowski
Copy link
Member

@dloucasfx, as I understand it, adding new resource attributes can be considered breaking because some backends use the full set of attributes to identify the time series. Therefore, additions can break continuity of data in some cases.

I chose mongodb_atlas.host.alias to keep the same pattern in place and to match what the naming the API is returning (ie: userAlias); especially that this field is documented internally and they can cross reference it with mongodb API docs.

I think you mean mongodb_atlas.user.alias, but in any case, our choice of naming is often different than what we find in APIs. We want to have attribute names that are consistent across a wide variety of technologies. This means we must often question the choices made by API authors. In this case, I think "user alias" implies that it is an alias for a user, while "host alias" correctly implies it is an alias for the host. Additionally, mongodb_atlas.host.alias falls into the mongodb_atlas.host.* namespace, which makes the name and alias fields co-discoverable.

@atoulme
Copy link
Contributor

atoulme commented Feb 22, 2023

Changes look good to me. I'm not sure about feature gating resource attributes - how best to warn then that we'll enable the resource attribute later? We can talk about it at our next SIG meeting.

@djaglowski
Copy link
Member

I'm not sure about feature gating resource attributes - how best to warn then that we'll enable the resource attribute later? We can talk about it at our next SIG meeting.

For some reason I thought this had come up in the past. Perhaps @dmitryax recalls whether this was the case? I don't personally feel strongly about it.

@dmitryax
Copy link
Member

dmitryax commented Mar 1, 2023

The document covers metric attributes. We still need to define how we handle resource attributes. I think it should be the same approach because most backends merge resource attributes with metric attributes, but this can be discussed separately.

We recently added the ability to have optional/default resource attributes. We can add this attribute as optional for now, and if we want to make it default, we can do that through the mdatagen built-in feature-gate system https://github.com/open-telemetry/opentelemetry-collector-contrib/blob/main/cmd/mdatagen/metric-metadata.yaml#L47-L49 (need to be added to resource attribute, but that's easy)

@dmitryax
Copy link
Member

dmitryax commented Mar 1, 2023

Created an issue to add the warnings in resource_attributes section #19174

@dloucasfx
Copy link
Contributor Author

@djaglowski @atoulme disabled the attribute and updated the changelog, please review the changes when you get a chance.
Thanks

…ute;the user-friendly hostname of the cluster node as displayed in the Atlas portal.

Signed-off-by: Dani Louca <[email protected]>
Copy link
Contributor

@schmikei schmikei left a comment

Choose a reason for hiding this comment

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

This looks good to me as well!

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

Successfully merging this pull request may close these issues.

5 participants