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

Request for additional destination.cloud.region field #1282

Closed
estolfo opened this issue Feb 23, 2021 · 16 comments
Closed

Request for additional destination.cloud.region field #1282

estolfo opened this issue Feb 23, 2021 · 16 comments
Labels
discuss enhancement New feature or request

Comments

@estolfo
Copy link

estolfo commented Feb 23, 2021

Hi ECS team

In working on a spec for the APM agents to instrument some AWS services, I've not found a field to record the AWS region.

I've proposed context.destination.region as a new field. I haven't worked on a spec for similar Azure services but we will have an Azure spec soon. I presume we would want to collect the region attribute somewhere for those as well.

Would it make sense to add a destination.region attribute to ECS?

Thanks!

@estolfo estolfo added the enhancement New feature or request label Feb 23, 2021
@ebeahan
Copy link
Member

ebeahan commented Feb 23, 2021

There's an existing field, cloud.region, which is intended to capture the cloud provider region. Would that fit your intended use case?

@axw
Copy link
Member

axw commented Feb 23, 2021

The difference here is that we're talking about the cloud in which a destination service (e.g. AWS S3) is running. Perhaps we could allow cloud to be nested under destination? That would permit recording this field as destination.cloud.region, for example.

@estolfo
Copy link
Author

estolfo commented Feb 24, 2021

And certain resources are associated with certain regions. For example, a bucket in s3 will be in a particular region, so the region is conceptually part of the destination.

@ebeahan
Copy link
Member

ebeahan commented Feb 24, 2021

The difference here is that we're talking about the cloud in which a destination service (e.g. AWS S3) is running.

I read through the linked spec, but I'm not fully grasping the difference in this intended use case. Is there an example event you could link to or share using what's proposed? Staying with AWS S3 as the example, I'm comparing some existing use of cloud.region such as in beats aws modules. I see value in staying consistent with the single field to capture the region across various event types. WDYT?

For this use case, would a source.cloud.region field ever be populated? Or would the cloud service/component always be the destination? Right now, destination.* and source.* contain identical fields by design to capture the same details for a source and destination of an exchange.

As a related side note, looking back at the description in the ECS docs for cloud.region (Region in which this host is running), we should improve the description. cloud.region can apply to other cloud resources beyond just hosts, like S3, SQS, SNS, etc.

@estolfo
Copy link
Author

estolfo commented Feb 25, 2021

Perhaps these tests in the ruby s3 client repo would help clarify what I'm trying to describe in the AWS spec. The user is able to create a S3 client configured with a particular region but it's not necessarily the same region where the client is running. Further, individual API requests may be made to interact with resources in different regions.

So given a client

client = Aws::S3::Client.new(region: 'us-west-2')

and a request

client.get_object(bucket: 'my-bucket', key: 'obj')

We would want to capture where the app with the ruby client is running in cloud.region and a destination as destination.cloud.region: 'us-west-2'.

With the same client, a request can be made as:

client.get_object(bucket: 'arn:aws:s3:us-east-1:123456789012:accesspoint:myendpoint', key: 'obj')

In this case, we would want to capture a destination as destination.cloud.region: 'us-east-1'. This is still not necessarily the same region as where the client is running (cloud.region), or the even region the client was configured with.

@estolfo estolfo changed the title Request for additional destination.region field Request for additional destination.cloud.region field Feb 26, 2021
@estolfo
Copy link
Author

estolfo commented Mar 4, 2021

Hi @ebeahan did you have a chance to read my previous comment?

@ebeahan
Copy link
Member

ebeahan commented Mar 4, 2021

@estolfo sorry for the delay.

We would want to capture where the app with the ruby client is running in cloud.region and a destination as destination.cloud.region: 'us-west-2'.

Thanks, the additional context here is helpful.

I still have a couple of outstanding concerns around destination.cloud.region.

  • While the destination.* naming makes sense for the use case described here, I still don't see it fitting into the existing destination.* ECS fieldset which is focused on a network exchange. Particularly, if we're not considering adding the same field to source.*.

    ECS has recently added the user.target.* concept for when an user is targeting another user in a particular event. There's a consideration for doing the same for process.* with process.target.*. Perhaps cloud.target.* makes sense when an action is targeting another region/cloud/etc?

  • Another concern, does this approach of using cloud.region as the "source" align with what other integrations are using today?

    @kaiyan-sheng @leehinman @adriansr would you be able to weigh in on how we handle this type of need today in integrations? For example, some integration runs in one region/cloud and collects data from a service/instance in another region/cloud? How are we typically populating cloud.region in those situations?

@beniwohli
Copy link

@ebeahan I agree that only adding destination.cloud.* introduces an asymmetry with source, but I don't understand why cloud.* is not fitting into the source / destination fieldsets in general. Wouldn't it be interesting for other uses cases as well to capture cloud-related metadata for cross-region network exchanges?

I've tried to find how e.g. beats uses cloud.region for cross-region requests, but due to being unfamiliar with the code base, wasn't really able to find something similar to the APM use case.

I guess another option could always be that we decide that this particular use case is too specific to APM and not standardize it within ECS.

@ebeahan
Copy link
Member

ebeahan commented Mar 31, 2021

The challenge around a [source|destination].cloud.* addition is how it fits into the established ECS paradigms.

  • The source.* and destination.* plus client.* and server.* fields are all very aligned to network flows. Adding other concepts ( like file modifications or process execs) has been discussed and source/destination was deemed better limited to network modeling, so I'm taking that into account here.
  • We do have the source.user.*/destination.user.* fields still in ECS, but there's also been discussion if that pattern should be deprecated in favor of user.* as the source and user.target.* as a destination. There's also some hesitation to add additional nestings under source/destination if it's a pattern we move away from.
  • Alignment with the existing top-level cloud.* fields need considering.
    • If we have both the top-level cloud.* and source.cloud.* and destination.cloud.*, what guidance do we give users for when to use one vs. the other?
    • What is the experience for users trying to query across data sources if some use cloud.* and others using [source]|[destination].cloud.*?

ECS has recently added the user.target.* concept for when a user is targeting another user in a particular event. There's a consideration for doing the same for process.* with process.target.*. Perhaps cloud.target.* makes sense when an action in one cloud region/provider is targeting another region/provider/etc?

Any thought about this as a possible approach?

@estolfo
Copy link
Author

estolfo commented Jun 15, 2021

Hi @ebeahan,
I'm back now from my leave so I'd like to work towards a conclusion on this discussion. All of your points are completely valid and it sounds like we (APM) have different semantics for some of the fields than you have. For example, we use destination for use cases other than network exchange. Because we already diverge on semantics, perhaps this use case is more specific to APM and it doesn't make sense to try to align. I'll check in with the rest of the team to gather input and get back to you.

@ebeahan
Copy link
Member

ebeahan commented Jul 20, 2021

@estolfo and I discussed the next steps outside this issue, and we considered the possibility of capturing the destination cloud as cloud.target.* as an RFC proposal.

I think the discussion has concluded here, but feel free to re-open if there's more to discuss.

@ebeahan ebeahan closed this as completed Jul 20, 2021
@simitt
Copy link
Contributor

simitt commented Oct 6, 2021

@ebeahan and @estolfo I couldn't find an RFC proposal for this. Has this been driven forward? We have an implementation issue open to add this new field (elastic/apm-server#5050).

@estolfo
Copy link
Author

estolfo commented Oct 6, 2021

@simitt I haven't written the proposal yet. Thanks for the ping- I can prioritize writing it so that the apm server can move forward with its open issue.

@estolfo
Copy link
Author

estolfo commented Oct 12, 2021

@simitt I've just learned that the fields are actually in beta, as they were incorporated into a larger RFC for AWS Lambda data. The proposal is in stage 2. I'm not sure if that means the server can go ahead with implementation. Here is the reference.

@simitt
Copy link
Contributor

simitt commented Oct 12, 2021

No pressure from apm-server side, was just interested in the current state. We will hold off with this for now.

@kgeller
Copy link
Contributor

kgeller commented Oct 12, 2021

@estolfo Stage 2 in ECS is intended to be for experimental use. If everyone is satisfied with how they are, we (ECS team) can help move forward with Stage 3.

ECS Stage Guidelines

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
discuss enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

6 participants