Fix for issue #99

[noamgat]

https://github.com/aws/aws-sdk-unity/issues/99

Storing global metrics and attributes in SessionStorage object to send them with the post-launch Stop / Resume session events

[karthiksaligrama]

Thanks for sending the pull request.

There seems to be some issues with the implementation. There are two types of global attributes/metrics which can be set on an event one is an event specific global attribute/metrics and other is a global attribute/metrics for all types of events. Your code seems to handle only the later.

[noamgat]

The resume / stop events wouldn't pick up on the event specific global attributes / metrics unless the user used their internal names ("_session.stop" etc) in the SetGlobalAttribute / Metric. Do you think its worth it to add support for this use case?

[karthiksaligrama]

I don't follow, why should the user have to do anything in this case? Also since your global event is serialized to a particular session which is not going to work for scenario's like a customer using multiple app id's.

[noamgat]

The reason to serialize the globals is so that the session resume / stop events contain their information if the app is killed and relaunched.

This leads to two questions: 1) Is a user expected to know the internal event names of the session resume / stop events? If not, they will not set global attributes / metrics for these specific events, and there is no need to serialize the event specific globals. 2) The multiple app id opens an interesting question - does the session storage concept support it at all? (Regardless of the global attributes). I don't think that the storage is stored on a per-app-id basis, so that is a different issue altogether.

[karthiksaligrama]

I think the reason to your 1st point is that your implementation ties up the serialization of global attrib/metric to session serialization which is not correct it should be serialized into its own store since it has no dependency with any session per say

to answer to 2 . Yes the session does support multiple app id serialization. Notice each session storage file is serialized into a file / appid

[noamgat]

Regarding 2, I see that now.

Regarding 1 - The way I see it, the question I'm trying to answer is "If the stop session event would have been sent when the app went to the background, which global attributes / metrics would have been sent along with it?" which is why the global attributes and metrics at the time of going to background should be stored together with the session info. This is why I think its correct to tie them together in this case and achieve that behavior.

On Thu, Jan 14, 2016 at 9:09 PM, Karthik Saligrama <notifications@github.com

wrote:

I think the reason to your 1st point is that your implementation ties up the serialization of global attrib/metric to session serialization which is not correct it should be serialized into its own store since it has no dependency with any session per say

to answer to 2 . Yes the session does support multiple app id serialization. Notice each session storage file is serialized into a file / appid

— Reply to this email directly or view it on GitHub https://github.com/aws/aws-sdk-unity/pull/115#issuecomment-171743507.

[karthiksaligrama]

I have a simpler question.

When do you set your global attributes? and can this be solved by simply setting the global attributes before you call session stop or resume? Something like

var ce =  new CustomEvent("dummyevent");
ce.AddGlobalAttributes("xyz","abc");
if(analyticsManager !=null)
{
    {
        analyticsManager.ResumeSession();
    }
    else
    {
        analyticsManager.PauseSession();
    }
}

[noamgat]

This is technically possible of course, but would force me (and others wanting global attributes and metrics in session stop events) to create a parallel storage object that contains the attributes. This opens up potential for the objects not being completely in sync and other serialization related errors if the storage is not the same. I think most users would like the ability to have their globals be reported in the session objects, so I think for reusability's sake its correct to have it as part of the framework.

On Thu, Jan 14, 2016 at 9:42 PM, Karthik Saligrama <notifications@github.com

wrote:

I have a simpler question.

When do you set your global attributes? and can this be solved by simply setting the global attributes before you call session stop or resume? Something like

var ce = new CustomEvent("dummyevent"); cr.AddGlobalAttributes("xyz","abc"); if(analyticsManager !=null) { { analyticsManager.ResumeSession(); } else { analyticsManager.PauseSession(); } }

— Reply to this email directly or view it on GitHub https://github.com/aws/aws-sdk-unity/pull/115#issuecomment-171756169.

[karthiksaligrama]

well ideally global events are supposed to be one time initialization when the application starts. I know this is not documented at the moment and i'll add those in our next release.

I agree it would make things easier for developers to not do that (reinitialize the global attribs/metrics), but i don't see how the developers could still get around not having to initialize the global attribs/metrics (e.g when the application is launched for the first time).

[karthiksaligrama]

I'm closing this pull request without merge, based on my comment above. If you have any questions or clarifications feel free to reopen/comment below