feat: add updated span events + trace more methods

[odeke-em]

This change carves out parts of PR #1241 in smaller pieces to ease with smaller reviews.
This change adds more span events, updates important spans to make them more distinct like changing:

"CloudSpanner.ReadWriteTransaction" to more direct and more pointed spans like:

• CloudSpanner.Transaction.execute_streaming_sql

Also added important spans:

• CloudSpanner.Database.run_in_transaction
• CloudSpanner.Session.run_in_transaction

[odeke-em]

@sakthivelmanii @harshachinta kindly help me run the bots. Thank you

[harshachinta]

@odeke-em
We should move this latest change of updating session last use time to beginning of this function, otherwise when opentlemetry is not installed then that update will never happen.

[harshachinta]

@odeke-em
this is not yet addresses.

[odeke-em]

I removed that @harshachinta as it is from the bigger PR and I don't think we shall need it anymore once some tests are sent in the other PR. I deleted it.

[harshachinta]

It is not from your PR, this was added by us to handle one case. Don't remove that
I am talking about this code. We need this to know when was a session last used to avoid making a backend call to verify session existence.

    if session:
        session._last_use_time = datetime.now()

[harshachinta]

I understand this requirement was bought out when we met last time.
Thinking out loud.

Cloud spanner has a mutation limit of 40000 in one request.
So lets say a customer adds mutations in a loop, then this will print 40,000 events on the span.
Is this a good user experience? Also this could add up to the storage cost on customer end?

[odeke-em]

That's a great question but it won't be a problem because:

• spans have a default event limit of 128 https://opentelemetry.io/docs/specs/otel/configuration/sdk-environment-variables/#span-limits
• storage costs not really as pricing on telemetry data is by span count rather than span data count unless perhps they have their own storage system but that's unlikely for those who'd be storing tens of thousands of span events

What we can do is perhaps document how to change span event limits and also document that we add events for span mutations.

[harshachinta]

@sakthivelmanii
I’d like to get your thoughts on this.

In my opinion, adding a separate event for every mutation could clutter the trace spans with too many events. Instead, I’d suggest avoiding one event per mutation. We already track the num_mutations attribute in the Commit request (here), which effectively provides customers with insights into the number of mutations in a commit.

Let me know what you think.

[harshachinta]

There is some mistake in ordering here. Can you check?
Ideally the order should be,

        "CloudSpanner.Database.run_in_transaction",
        "CloudSpanner.CreateSession",
        "CloudSpanner.Session.run_in_transaction",
        "CloudSpanner.Transaction.execute_streaming_sql",
        "CloudSpanner.Transaction.execute_streaming_sql",
        "CloudSpanner.Transaction.commit",
        ```

[odeke-em]

It isn't a mistake though but rather the order in which spans are ended @harshachinta.

[harshachinta]

@sakthivelmanii
I’d like to get your thoughts on this.

In my opinion, adding a separate event for every mutation could clutter the trace spans with too many events. Instead, I’d suggest avoiding one event per mutation. We already track the num_mutations attribute in the Commit request (here), which effectively provides customers with insights into the number of mutations in a commit.

Let me know what you think.

[harshachinta]

why is there a repeation of exception event?

[odeke-em]

So given that we have a direct invocation of with trace_call("testBind") per

        try: 
            with trace_call("testBind", fauxSession):
                pool.bind(database)
        except Exception:
            pass 

that then invokes code internally where an exception happens. With the exception re-raised and unhandled until the highest parent, OpenTelemetry records the exception even on the parent. If we swallowed the exception internally we wouldn't