Add shared spec for span type/subtype

[SylvainJuge]

Adds a JSON spec + document status quo for known sub-types

Aims to document the status-quo, thus most span types & subtypes that are used by any agent should be reflected here.
Enforcing spec in tests should allow to opt-out for exact match to allow testing for:

• test-only values (like when not testing against a real database that actually exists)
• user-provided entries that are free-form

Things to discuss

• creating & moving dedicated spec for storage type(s) ? >> will be handled by follow-up issue
• adding dedicated section for Azure services (similar to AWS) ? Shoud contain a section for Azure CosmosDB, File Storage, Blob Storage: Add spec for instrumenting Azure services #426 partially covers that
• type / subtype values that may be Java-only for now, but might be relevant later: should we include them right now ?
  • app / inferred for inferred spans captured through profiling
  • process for external command execution
  • custom / (anything) for spans created through API/Annotations without explicit type set
• Should platform-specific values for type / subtype be part of this shared spec ?
  • having everything in this spec would make it easier to "being able to put a dedicated UI on every span type"
  • db / h2 database is Java-only
  • template used for template engines is really platform-specific, but the template type should be common
  • db / hibernate-search is Java-only
• Bonus: should we have a dedicated orm type ?>> will be handled by follow-up issue
  • For example to have orm / hibernate-search instead of db / hibernate-search
  • Given it's platform-specific, it would likely be like template type.

---

Steps

• commit initial draft spec format + entries for Java agent
• integrate changes from another agent : Ruby @elastic/apm-agent-ruby (Validate span.type and .subtype against shared spec apm-agent-ruby#1030)
• get review for a few other agents + merge it
• open follow-up issues/PRs to align inconsistencies & provide enhancements, known inconsistencies Alignment of span type/subtype #502

[apmmachine]

💚 Build Succeeded

the below badges are clickable and redirect to their specific view in the CI or DOCS

Expand to view the summary

Build stats

• Start Time: 2021-09-06T04:45:00.397+0000

• Duration: 6 min 0 sec

• Commit: c6c38f5

Trends 🧪

[felixbarny]

values that may be Java-only for now, but might be relevant later: should we include them right now ?

I'd say yes. Even if something seems language-specific now, it can be relevant for others in the future. It also forces to think about how to make things generic enough so it may be used by other languages. We'll probably amend this list with other agent-specific types/subtypes.

Should platform-specific values for type / subtype be part of this shared spec ?

I'd err on the side of including platform-specific type/subtypes. If it turns out that updating the spec becomes too frequent and tedious, we can think about allowing any subtypes of a type.

Overall, I think it's fine to include platform-specific subtypes. It gives a good overview on what we support and helps to avoid subtle differences in the subtypes we use in the future.

[SylvainJuge]

Main PR description has been updated to reflect our current aim: document the status quo and make a bit more obvious that further alignment issues/PRs will be handled later.

[basepi]

My only nit: I'm unsure that "__used_by" is actually useful. It will be hard to keep up to date and whether an agent is using it yet or not doesn't seem to me to matter much.

[SylvainJuge]

My only nit: I'm unsure that "__used_by" is actually useful. It will be hard to keep up to date and whether an agent is using it yet or not doesn't seem to me to matter much.

I agree with you, it does not matter at all when enforcing the spec, but the purpose is more about documentation & help with alignment, which will come next:

• what is the most commonly used convention ?
• how many agents are impacted by doing any change ?
• how to prioritize follow-up alignment tasks ?

Once we have proper alignment, there won't be any need to keep it, thus it's only temporary.

[astorm]

@SylvainJuge Thanks for taking this on.

I did a quick survey of the types and subtypes in the Node.js Agent and I discovered a few discrepancies with what's in this spec PR.

First -- there's two types in the Node.js Agent that aren't present in span_types.json

Type:     websocket
Subtypes: send

Type:     cache
Subtypes: redis

One is for instrumenting the sending of websocket message via https://www.npmjs.com/package/ws

The other appears to be a place where the Node.js has placed its redis spans under a cache type instead of a db type. I'm unsure of the history here.

Second -- there's three db subtypes from the Node.js Agent that are missing from span_types.json.

Type:       db
Subtypes:   mssql, memcached, graphql

The Node.js uses the memcached subtype for instrumenting memcached operations performed by: https://www.npmjs.com/package/memcached

The Node.js uses the mssql subtype for instrumenting database operations performed by: https://www.npmjs.com/package/mssql

The Node.js uses the graphql subtype for instrumenting graphql queries performed by https://www.npmjs.com/package/graphql (a common library used by different graphql implementations in Node.js)

If we're trying to have this reflect the current reality we'll want to get span_types.json updated.

[trentm]

For some comparison, it looks like the Python APM agent uses:

• type=cache for memcached: https://github.com/elastic/apm-agent-python/blob/master/elasticapm/instrumentation/packages/pymemcache.py#L95-L107 and https://github.com/elastic/apm-agent-python/blob/master/elasticapm/instrumentation/packages/pylibmc.py#L72-L74 and
• type=db for redis: https://github.com/elastic/apm-agent-python/blob/master/elasticapm/instrumentation/packages/redis.py#L65 and https://github.com/elastic/apm-agent-python/blob/master/elasticapm/instrumentation/packages/asyncio/aioredis.py#L51

[SylvainJuge]

Thanks @astorm and @trentm for the feedback:

I've added websocket/send and db/graphql as-is to the spec.

For datastores that could be used both as a cache of a database like memcache or redis I think we should keep them in db:

• usage as a cache or as a database is application-dependant
• would be consistent with OpenTelemetry specification

I don't think it's worth adding duplicate entries for {cache,db}/{redis,memcache}, as it would be easy to fix once the JSON spec is enforced in each agent. But we can also add them to make merging this PR easier, and then deal with the inconsistency in a follow-up PR.

For db/mssql, there is already the db/sqlserver that is used by the Java agent, we can definitely use mssql as it's also consistent with OTel spec (I've added it and listed in the known inconsistencies in PR description).

[trentm]

For datastores that could be used both as a cache of a database like memcache or redis I think we should keep them in db:

I agree. (My guess is that this shouldn't be considered a compatibility issue if we change it, but I'm curious if others have opinions on that.)

[basepi]

When we talk about "enforcing" this spec, what does that mean? Checking any type/subtype we use against this list to make sure it's "approved"? Just wanted to clarify before I open the Python issue for aligning.

[SylvainJuge]

When we talk about "enforcing" this spec, what does that mean? Checking any type/subtype we use against this list to make sure it's "approved"? Just wanted to clarify before I open the Python issue for aligning.

Yes, exactly this: enforcing the shared spec.

Once checks starts to be enforced, the fun part of doing alignment can start :-).

[SylvainJuge]

Merging spec as-is for now.

ping @elastic/apm-agent-net to create an issue/PR to enforce the JSON specification .